From e95c59cd2f0eb0ac4885dc46e7529193d430a08d Mon Sep 17 00:00:00 2001 From: Andres Notzli Date: Tue, 28 Jul 2015 16:41:40 -0700 Subject: [PATCH] Count number of corrupt keys during compaction Summary: For task #7771355, we would like to log the number of corrupt keys during a compaction. This patch implements and tests the count as part of CompactionJobStats. Test Plan: make && make check Reviewers: rven, igor, yhchiang, sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D42921 --- db/compaction_job.cc | 4 ++++ db/compaction_job_stats_test.cc | 6 ++++++ db/compaction_job_test.cc | 23 ++++++++++++----------- include/rocksdb/compaction_job_stats.h | 5 ++++- util/compaction_job_stats_impl.cc | 17 +++++++++-------- 5 files changed, 35 insertions(+), 20 deletions(-) diff --git a/db/compaction_job.cc b/db/compaction_job.cc index 1f924e9bb..2165044a9 100644 --- a/db/compaction_job.cc +++ b/db/compaction_job.cc @@ -385,6 +385,10 @@ Status CompactionJob::ProcessKeyValueCompaction(int64_t* imm_micros, has_current_user_key = false; last_sequence_for_key = kMaxSequenceNumber; visible_in_snapshot = kMaxSequenceNumber; + + if (compaction_job_stats_ != nullptr) { + compaction_job_stats_->num_corrupt_keys++; + } } else { if (compaction_job_stats_ != nullptr && ikey.type == kTypeDeletion) { compaction_job_stats_->num_input_deletion_records++; diff --git a/db/compaction_job_stats_test.cc b/db/compaction_job_stats_test.cc index 1c066e8e9..e61dfc2c5 100644 --- a/db/compaction_job_stats_test.cc +++ b/db/compaction_job_stats_test.cc @@ -467,6 +467,9 @@ class CompactionJobStatsChecker : public EventListener { ASSERT_EQ(current_stats.num_records_replaced, stats.num_records_replaced); + ASSERT_EQ(current_stats.num_corrupt_keys, + stats.num_corrupt_keys); + ASSERT_EQ( std::string(current_stats.smallest_output_key_prefix), std::string(stats.smallest_output_key_prefix)); @@ -509,6 +512,9 @@ class CompactionJobDeletionStatsChecker : public CompactionJobStatsChecker { ASSERT_EQ( current_stats.num_records_replaced, stats.num_records_replaced); + + ASSERT_EQ(current_stats.num_corrupt_keys, + stats.num_corrupt_keys); } }; diff --git a/db/compaction_job_test.cc b/db/compaction_job_test.cc index ab716c068..781c0dd7b 100644 --- a/db/compaction_job_test.cc +++ b/db/compaction_job_test.cc @@ -46,16 +46,14 @@ void VerifyInitializationOfCompactionJobStats( ASSERT_EQ(compaction_job_stats.largest_output_key_prefix[0], 0); ASSERT_EQ(compaction_job_stats.num_records_replaced, 0U); + + ASSERT_EQ(compaction_job_stats.num_input_deletion_records, 0U); + ASSERT_EQ(compaction_job_stats.num_expired_deletion_records, 0U); + + ASSERT_EQ(compaction_job_stats.num_corrupt_keys, 0U); #endif // !defined(IOS_CROSS_COMPILE) } -void VerifyCompactionJobStats(const CompactionJobStats& compaction_job_stats, - const std::vector& files, - size_t num_output_files) { - ASSERT_GE(compaction_job_stats.elapsed_micros, 0U); - ASSERT_EQ(compaction_job_stats.num_input_files, files.size()); - ASSERT_EQ(compaction_job_stats.num_output_files, num_output_files); -} } // namespace // TODO(icanadi) Make it simpler once we mock out VersionSet @@ -197,13 +195,12 @@ class CompactionJobTest : public testing::Test { LogBuffer log_buffer(InfoLogLevel::INFO_LEVEL, db_options_.info_log.get()); mutex_.Lock(); EventLogger event_logger(db_options_.info_log.get()); - CompactionJobStats compaction_job_stats; CompactionJob compaction_job( 0, &compaction, db_options_, env_options_, versions_.get(), &shutting_down_, &log_buffer, nullptr, nullptr, nullptr, {}, - table_cache_, &event_logger, false, dbname_, &compaction_job_stats); + table_cache_, &event_logger, false, dbname_, &compaction_job_stats_); - VerifyInitializationOfCompactionJobStats(compaction_job_stats); + VerifyInitializationOfCompactionJobStats(compaction_job_stats_); compaction_job.Prepare(); mutex_.Unlock(); @@ -214,7 +211,9 @@ class CompactionJobTest : public testing::Test { ASSERT_OK(s); mutex_.Unlock(); - VerifyCompactionJobStats(compaction_job_stats, files, 1); + ASSERT_GE(compaction_job_stats_.elapsed_micros, 0U); + ASSERT_EQ(compaction_job_stats_.num_input_files, files.size()); + ASSERT_EQ(compaction_job_stats_.num_output_files, 1U); } Env* env_; @@ -230,6 +229,7 @@ class CompactionJobTest : public testing::Test { InstrumentedMutex mutex_; std::atomic shutting_down_; std::shared_ptr mock_table_factory_; + CompactionJobStats compaction_job_stats_; }; TEST_F(CompactionJobTest, Simple) { @@ -248,6 +248,7 @@ TEST_F(CompactionJobTest, SimpleCorrupted) { auto files = cfd->current()->storage_info()->LevelFiles(0); RunCompaction(files); + ASSERT_EQ(compaction_job_stats_.num_corrupt_keys, 400U); mock_table_factory_->AssertLatestFile(expected_results); } diff --git a/include/rocksdb/compaction_job_stats.h b/include/rocksdb/compaction_job_stats.h index 50bbdab33..3eff7a006 100644 --- a/include/rocksdb/compaction_job_stats.h +++ b/include/rocksdb/compaction_job_stats.h @@ -49,12 +49,15 @@ struct CompactionJobStats { // the number of deletion entries before compaction. Deletion entries // can disappear after compaction because they expired uint64_t num_input_deletion_records; - // number of deletion records that were found obsolete and discarded // because it is not possible to delete any more keys with this entry // (i.e. all possible deletions resulting from it have been completed) uint64_t num_expired_deletion_records; + // number of corrupt keys (ParseInternalKey returned false when applied to + // the key) encountered and written out. + uint64_t num_corrupt_keys; + // 0-terminated strings storing the first 8 bytes of the smallest and // largest key in the output. static const size_t kMaxPrefixLength = 8; diff --git a/util/compaction_job_stats_impl.cc b/util/compaction_job_stats_impl.cc index 2496b1097..fd60d8abe 100644 --- a/util/compaction_job_stats_impl.cc +++ b/util/compaction_job_stats_impl.cc @@ -3,8 +3,7 @@ // LICENSE file in the root directory of this source tree. An additional grant // of patent rights can be found in the PATENTS file in the same directory. -#include -#include "include/rocksdb/compaction_job_stats.h" +#include "rocksdb/compaction_job_stats.h" namespace rocksdb { @@ -13,25 +12,27 @@ namespace rocksdb { void CompactionJobStats::Reset() { elapsed_micros = 0; + num_input_records = 0; num_input_files = 0; num_input_files_at_output_level = 0; + + num_output_records = 0; num_output_files = 0; - num_input_records = 0; - num_output_records = 0; + is_manual_compaction = 0; total_input_bytes = 0; total_output_bytes = 0; + num_records_replaced = 0; + total_input_raw_key_bytes = 0; total_input_raw_value_bytes = 0; - num_records_replaced = 0; - - is_manual_compaction = 0; - num_input_deletion_records = 0; num_expired_deletion_records = 0; + + num_corrupt_keys = 0; } #else