Fix compaction filter test

Signed-off-by: Yang Zhang <[email protected]>

db/compact_files_test.cc

@@ -64,21 +64,23 @@ class TestFilterFactory : public CompactionFilterFactory {
  public:
   std::shared_ptr<CompactionFilter::Context> context_;
   std::shared_ptr<int> compaction_count_;
+  std::string start_key_;
+  std::string end_key_;
 
   TestFilterFactory(std::shared_ptr<CompactionFilter::Context> context,
-                    std::shared_ptr<int> compaction_count) {
-    this->context_ = context;
-    this->compaction_count_ = compaction_count;
-  }
+                    std::shared_ptr<int> compaction_count)
+      : context_(context), compaction_count_(compaction_count) {}
 
   ~TestFilterFactory() {}
 
   const char* Name() const { return "TestFilterFactory"; }
 
   std::unique_ptr<CompactionFilter> CreateCompactionFilter(
       const CompactionFilter::Context& context) {
-    context_->start_key = context.start_key;
-    context_->end_key = context.end_key;
+    start_key_ = context.start_key.ToString();
+    context_->start_key = Slice(start_key_);
+    end_key_ = context.end_key.ToString();
+    context_->end_key = Slice(end_key_);
     context_->is_end_key_inclusive = context.is_end_key_inclusive;
     context_->input_table_properties = context.input_table_properties;
     *compaction_count_.get() += 1;
@@ -115,16 +117,16 @@ TEST_F(CompactFilesTest, FilterContext) {
   assert(db);
 
   // `Flush` is different from `Compaction`.
-  db->Put(WriteOptions(), std::to_string(1), "");
-  db->Put(WriteOptions(), std::to_string(51), "");
+  db->Put(WriteOptions(), "1", "");
+  db->Put(WriteOptions(), "51", "");
   db->Flush(FlushOptions());
-  db->Put(WriteOptions(), std::to_string(50), "");
-  db->Put(WriteOptions(), std::to_string(99), "");
+  db->Put(WriteOptions(), "50", "");
+  db->Put(WriteOptions(), "99", "");
   db->Flush(FlushOptions());
   ASSERT_EQ(*compaction_count.get(), 0);
 
   db->CompactRange(CompactRangeOptions(), nullptr, nullptr);
-  usleep(10000);  // Wait for compaction start.
+  ASSERT_OK(static_cast_with_check<DBImpl>(db)->TEST_WaitForCompact());
   ASSERT_EQ(expected_context->start_key, Slice("1"));
   ASSERT_EQ(expected_context->is_end_key_inclusive, 1);
   ASSERT_EQ(expected_context->input_table_properties.size(), 2);
@@ -608,7 +610,7 @@ TEST_F(CompactFilesTest, IsWriteStalled) {
     for (int j = 0; j < 100; ++j) {
       char key[16];
       bzero(key, 16);
-      sprintf(key, "foo%.2d", j);
+      snprintf(key, 5, "foo%.2d", j);
       ASSERT_OK(wb.Put(handles[0], key, "bar"));
     }
 

db/compaction/compaction.cc

@@ -814,7 +814,7 @@ uint64_t Compaction::OutputFilePreallocationSize() const {
 }
 
 std::unique_ptr<CompactionFilter> Compaction::CreateCompactionFilter(
-    std::optional<Slice> start, std::optional<Slice> end) const {
+    const std::optional<Slice>& start, const std::optional<Slice>& end) const {
   if (!cfd_->ioptions()->compaction_filter_factory) {
     return nullptr;
   }
@@ -833,11 +833,11 @@ std::unique_ptr<CompactionFilter> Compaction::CreateCompactionFilter(
   context.reason = TableFileCreationReason::kCompaction;
   context.input_table_properties = GetInputTableProperties();
   context.is_bottommost_level = bottommost_level_;
-  context.start_key = start == std::nullopt ? GetSmallestUserKey()
-                                            : ExtractUserKey(start.value());
+  context.start_key =
+      start.has_value() ? ExtractUserKey(*start) : GetSmallestUserKey();
   context.end_key =
-      end == std::nullopt ? GetLargestUserKey() : ExtractUserKey(end.value());
-  context.is_end_key_inclusive = end == std::nullopt;
+      end.has_value() ? ExtractUserKey(*end) : GetLargestUserKey();
+  context.is_end_key_inclusive = !end.has_value();
   if (context.input_table_properties.empty()) {
     ROCKS_LOG_WARN(
         immutable_options_.info_log,

db/compaction/compaction.h

@@ -312,7 +312,7 @@ class Compaction {
 
   // Create a CompactionFilter from compaction_filter_factory
   std::unique_ptr<CompactionFilter> CreateCompactionFilter(
-      std::optional<Slice> start, std::optional<Slice> end) const;
+      const std::optional<Slice>& start, const std::optional<Slice>& end) const;
 
   // Create a SstPartitioner from sst_partitioner_factory
   std::unique_ptr<SstPartitioner> CreateSstPartitioner() const;

db/db_compaction_test.cc

@@ -247,7 +247,7 @@ class SplitAllPartitioner : public SstPartitioner {
     return PartitionerResult::kRequired;
   }
 
-  bool CanDoTrivialMove(const Slice&, const Slice&) { return true; }
+  bool CanDoTrivialMove(const Slice&, const Slice&) override { return true; }
 };
 
 class SplitAllPatitionerFactory : public SstPartitionerFactory {
@@ -1226,9 +1226,9 @@ TEST_F(DBCompactionTest, CompactionSstPartitionerNextLevel) {
           ASSERT_EQ(next_level_overlap_files, cx.OutputNextLevelSegmentCount());
           for (size_t i = 0; i < overlapped_files.size(); i++) {
             Slice next_level_lower, next_level_upper;
-            int next_level_size;
-            cx.OutputNextLevelSegment(i, &next_level_lower, &next_level_upper,
-                                      &next_level_size);
+            size_t next_level_size;
+            cx.OutputNextLevelSegment((int)i, &next_level_lower,
+                                      &next_level_upper, &next_level_size);
 
             if (i == 0) {
               ASSERT_EQ(overlapped_files[i].smallestkey, next_level_lower);
@@ -1243,24 +1243,24 @@ TEST_F(DBCompactionTest, CompactionSstPartitionerNextLevel) {
   ASSERT_OK(Put("A", "there are more than 10 bytes."));
   ASSERT_OK(Put("B", "yet another key."));
   ASSERT_OK(Flush());
-  ASSERT_OK(dbfull()->TEST_WaitForCompact(true));
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
   ASSERT_OK(Put("A1", "the new challenger..."));
   ASSERT_OK(Put("B1", "and his buddy."));
   ASSERT_OK(Flush());
   ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable());
-  ASSERT_OK(dbfull()->TEST_WaitForCompact(true));
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
   ASSERT_OK(Put("A1P", "the new challenger... Changed."));
   ASSERT_OK(Put("B1P", "and his buddy. Changed too."));
   ASSERT_OK(Flush());
   ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable());
-  ASSERT_OK(dbfull()->TEST_WaitForCompact(true));
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
   ASSERT_OK(Put(InternalKey("A", 0, ValueType::kTypeDeletion).Encode(),
                 "And a tricker: he pretends to be A, but not A."));
   ASSERT_OK(Put(InternalKey("B", 0, ValueType::kTypeDeletion).Encode(),
                 "Yeah, another tricker."));
   ASSERT_OK(Flush());
   ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable());
-  ASSERT_OK(dbfull()->TEST_WaitForCompact(true));
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
 
   std::vector<LiveFileMetaData> files;
   dbfull()->GetLiveFilesMetaData(&files);