From bb808eaddb04fde7837721528a3fe2f2f6fee168 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Thu, 4 Jun 2015 12:31:12 -0700 Subject: [PATCH] Changed the CompactionJobStats::output_key_prefix type from char[] to string. Summary: Keys in RocksDB can be arbitrary byte strings. However, in the current CompactionJobStats, smallest_output_key_prefix and largest_output_key_prefix are of type char[] without having a length, which is insufficient to handle non-null terminated strings. This patch change their type to std::string. Test Plan: compaction_job_stats_test Reviewers: igor, rven, IslamAbdelRahman, kradhakrishnan, sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D39537 --- db/compaction_job.cc | 21 ++++++++++----------- db/compaction_job_stats_test.cc | 21 ++++++++++----------- include/rocksdb/compaction_job_stats.h | 7 +++++-- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/db/compaction_job.cc b/db/compaction_job.cc index 13ada27de..46680aa84 100644 --- a/db/compaction_job.cc +++ b/db/compaction_job.cc @@ -1252,11 +1252,10 @@ void CompactionJob::CleanupCompaction(const Status& status) { #ifndef ROCKSDB_LITE namespace { void CopyPrefix( - char* dst, size_t dst_length, const Slice& src) { - assert(dst_length > 0); - size_t length = src.size() > dst_length - 1 ? dst_length - 1 : src.size(); - memcpy(dst, src.data(), length); - dst[length] = 0; + const Slice& src, size_t prefix_length, std::string* dst) { + assert(prefix_length > 0); + size_t length = src.size() > prefix_length ? prefix_length : src.size(); + dst->assign(src.data(), length); } } // namespace @@ -1285,13 +1284,13 @@ void CompactionJob::UpdateCompactionJobStats( if (compact_->outputs.size() > 0U) { CopyPrefix( - compaction_job_stats_->smallest_output_key_prefix, - sizeof(compaction_job_stats_->smallest_output_key_prefix), - compact_->outputs[0].smallest.user_key().ToString()); + compact_->outputs[0].smallest.user_key(), + CompactionJobStats::kMaxPrefixLength, + &compaction_job_stats_->smallest_output_key_prefix); CopyPrefix( - compaction_job_stats_->largest_output_key_prefix, - sizeof(compaction_job_stats_->largest_output_key_prefix), - compact_->current_output()->largest.user_key().ToString()); + compact_->current_output()->largest.user_key(), + CompactionJobStats::kMaxPrefixLength, + &compaction_job_stats_->largest_output_key_prefix); } } #endif // !ROCKSDB_LITE diff --git a/db/compaction_job_stats_test.cc b/db/compaction_job_stats_test.cc index bd666ba05..678fbf262 100644 --- a/db/compaction_job_stats_test.cc +++ b/db/compaction_job_stats_test.cc @@ -457,11 +457,10 @@ uint64_t EstimatedFileSize( namespace { void CopyPrefix( - char* dst, size_t dst_length, const Slice& src) { - assert(dst_length > 0); - size_t length = src.size() > dst_length - 1 ? dst_length - 1 : src.size(); - memcpy(dst, src.data(), length); - dst[length] = 0; + const Slice& src, size_t prefix_length, std::string* dst) { + assert(prefix_length > 0); + size_t length = src.size() > prefix_length ? prefix_length : src.size(); + dst->assign(src.data(), length); } } // namespace @@ -499,12 +498,12 @@ CompactionJobStats NewManualCompactionJobStats( stats.num_records_replaced = num_records_replaced; - CopyPrefix(stats.smallest_output_key_prefix, - sizeof(stats.smallest_output_key_prefix), - smallest_key); - CopyPrefix(stats.largest_output_key_prefix, - sizeof(stats.largest_output_key_prefix), - largest_key); + CopyPrefix(smallest_key, + CompactionJobStats::kMaxPrefixLength, + &stats.smallest_output_key_prefix); + CopyPrefix(largest_key, + CompactionJobStats::kMaxPrefixLength, + &stats.largest_output_key_prefix); return stats; } diff --git a/include/rocksdb/compaction_job_stats.h b/include/rocksdb/compaction_job_stats.h index c7119dd19..7c05c827e 100644 --- a/include/rocksdb/compaction_job_stats.h +++ b/include/rocksdb/compaction_job_stats.h @@ -6,6 +6,7 @@ #pragma once #include #include +#include namespace rocksdb { struct CompactionJobStats { @@ -45,7 +46,9 @@ struct CompactionJobStats { // 0-terminated strings storing the first 8 bytes of the smallest and // largest key in the output. - char smallest_output_key_prefix[9]; - char largest_output_key_prefix[9]; + static const size_t kMaxPrefixLength = 8; + + std::string smallest_output_key_prefix; + std::string largest_output_key_prefix; }; } // namespace rocksdb