diff --git a/HISTORY.md b/HISTORY.md index be4afeb95..bec7a56a7 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -14,6 +14,7 @@ * Fix a bug of wrong iterator result if another thread finishes an update and a DB flush between two statement. * Disable file deletion after MANIFEST write/sync failure until db re-open or Resume() so that subsequent re-open will not see MANIFEST referencing deleted SSTs. * Fix a bug when index_type == kTwoLevelIndexSearch in PartitionedIndexBuilder to update FlushPolicy to point to internal key partitioner when it changes from user-key mode to internal-key mode in index partition. +* Make compaction report InternalKey corruption while iterating over the input. ### Public API Change * `DB::GetDbSessionId(std::string& session_id)` is added. `session_id` stores a unique identifier that gets reset every time the DB is opened. This DB session ID should be unique among all open DB instances on all hosts, and should be unique among re-openings of the same or other DBs. This identifier is recorded in the LOG file on the line starting with "DB Session ID:". diff --git a/db/compaction/compaction_iterator.cc b/db/compaction/compaction_iterator.cc index 6714b3473..94873255b 100644 --- a/db/compaction/compaction_iterator.cc +++ b/db/compaction/compaction_iterator.cc @@ -263,6 +263,7 @@ void CompactionIterator::NextFromInput() { iter_stats_.num_input_records++; if (!ParseInternalKey(key_, &ikey_)) { + iter_stats_.num_input_corrupt_records++; // If `expect_valid_internal_key_` is false, return the corrupted key // and let the caller decide what to do with it. // TODO(noetzli): We should have a more elegant solution for this. @@ -275,7 +276,6 @@ void CompactionIterator::NextFromInput() { has_current_user_key_ = false; current_user_key_sequence_ = kMaxSequenceNumber; current_user_key_snapshot_ = 0; - iter_stats_.num_input_corrupt_records++; valid_ = true; break; } diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index 50ed5aeb6..249fb93d8 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -898,9 +898,10 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) { sub_compact->c_iter.reset(new CompactionIterator( input.get(), cfd->user_comparator(), &merge, versions_->LastSequence(), &existing_snapshots_, earliest_write_conflict_snapshot_, - snapshot_checker_, env_, ShouldReportDetailedTime(env_, stats_), false, - &range_del_agg, sub_compact->compaction, compaction_filter, - shutting_down_, preserve_deletes_seqnum_, manual_compaction_paused_, + snapshot_checker_, env_, ShouldReportDetailedTime(env_, stats_), + /*expect_valid_internal_key=*/true, &range_del_agg, + sub_compact->compaction, compaction_filter, shutting_down_, + preserve_deletes_seqnum_, manual_compaction_paused_, db_options_.info_log)); auto c_iter = sub_compact->c_iter.get(); c_iter->SeekToFirst(); diff --git a/db/compaction/compaction_job_test.cc b/db/compaction/compaction_job_test.cc index 708ca0c21..aeffc8d1e 100644 --- a/db/compaction/compaction_job_test.cc +++ b/db/compaction/compaction_job_test.cc @@ -395,7 +395,7 @@ TEST_F(CompactionJobTest, Simple) { RunCompaction({ files }, expected_results); } -TEST_F(CompactionJobTest, SimpleCorrupted) { +TEST_F(CompactionJobTest, DISABLED_SimpleCorrupted) { NewDB(); auto expected_results = CreateTwoFiles(true); @@ -989,7 +989,7 @@ TEST_F(CompactionJobTest, MultiSingleDelete) { // single deletion and the (single) deletion gets removed while the corrupt key // gets written out. TODO(noetzli): We probably want a better way to treat // corrupt keys. -TEST_F(CompactionJobTest, CorruptionAfterDeletion) { +TEST_F(CompactionJobTest, DISABLED_CorruptionAfterDeletion) { NewDB(); auto file1 =