Age should be sorted.

facebook · May 11, 2023 · 042c4cb · 042c4cb
1 parent e85123d
commit 042c4cb
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 6 deletions.
diff --git a/db/column_family.cc b/db/column_family.cc
@@ -1435,6 +1435,30 @@ Status ColumnFamilyData::ValidateOptions(
         "or 8 bytes per key.");
   }
 
+  if (!cf_options.compaction_options_fifo.file_temperature_age_thresholds
+           .empty()) {
+    if (cf_options.compaction_style != kCompactionStyleFIFO) {
+      return Status::NotSupported(
+          "Option file_temperature_age_thresholds only supports FIFO "
+          "compaction.");
+    } else if (cf_options.num_levels > 1) {
+      return Status::NotSupported(
+          "Option file_temperature_age_thresholds is only supported when "
+          "num_levels = 1.");
+    } else {
+      const auto& ages =
+          cf_options.compaction_options_fifo.file_temperature_age_thresholds;
+      assert(ages.size() >= 1);
+      // check that age is sorted
+      for (size_t i = 0; i < ages.size() - 1; ++i) {
+        if (ages[i].age >= ages[i + 1].age) {
+          return Status::NotSupported(
+              "Option file_temperature_age_thresholds requires elements to be "
+              "sorted in increasing order with respect to `age` field.");
+        }
+      }
+    }
+  }
   if (cf_options.compaction_style == kCompactionStyleFIFO) {
     if (cf_options.num_levels > 1 &&
         !cf_options.compaction_options_fifo.file_temperature_age_thresholds

diff --git a/db/db_options_test.cc b/db/db_options_test.cc
@@ -1236,26 +1236,48 @@ TEST_F(DBOptionsTest, BottommostCompressionOptsWithFallbackType) {
   ASSERT_EQ(kBottommostCompressionLevel, compression_opt_used.level);
 }
 
-TEST_F(DBOptionsTest, FIFOTemperatureAgeThresholdSingleLevel) {
+TEST_F(DBOptionsTest, FIFOTemperatureAgeThresholdValidation) {
   Options options = CurrentOptions();
   Destroy(options);
 
   options.num_levels = 1;
   options.compaction_style = kCompactionStyleFIFO;
+  options.max_open_files = -1;
+  // elements are not sorted
+  // During DB open
   options.compaction_options_fifo.file_temperature_age_thresholds.push_back(
       {Temperature::kCold, 1000});
-  options.max_open_files = -1;
+  options.compaction_options_fifo.file_temperature_age_thresholds.push_back(
+      {Temperature::kWarm, 500});
+  Status s = TryReopen(options);
+  ASSERT_TRUE(s.IsNotSupported());
+  ASSERT_TRUE(std::strstr(
+      s.getState(),
+      "Option file_temperature_age_thresholds requires elements to be sorted "
+      "in increasing order with respect to `age` field."));
+  // Dynamically set option
+  options.compaction_options_fifo.file_temperature_age_thresholds.pop_back();
   ASSERT_OK(TryReopen(options));
-  // During DB Open
+  s = db_->SetOptions({{"compaction_options_fifo",
+                        "{file_temperature_age_thresholds={{temperature=kCold;"
+                        "age=1000000}:{temperature=kWarm;age=1}}}"}});
+  ASSERT_TRUE(s.IsNotSupported());
+  ASSERT_TRUE(std::strstr(
+      s.getState(),
+      "Option file_temperature_age_thresholds requires elements to be sorted "
+      "in increasing order with respect to `age` field."));
+
+  // not single level
+  // During DB open
   options.num_levels = 2;
-  Status s = TryReopen(options);
+  s = TryReopen(options);
   ASSERT_TRUE(s.IsNotSupported());
   ASSERT_TRUE(std::strstr(s.getState(),
                           "Option file_temperature_age_thresholds is only "
                           "supported when num_levels = 1."));
+  // Dynamically set option
   options.compaction_options_fifo.file_temperature_age_thresholds.clear();
   DestroyAndReopen(options);
-  // Dynamically set option
   s = db_->SetOptions(
       {{"compaction_options_fifo",
         "{file_temperature_age_thresholds={temperature=kCold;age=1000}}"}});

diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h
@@ -240,7 +240,7 @@ struct CompactionOptionsFIFO {
   // Dynamically changeable through SetOptions() API, e.g.,
   //   SetOptions("compaction_options_fifo",
   //   "{file_temperature_age_thresholds={
-  //    {age=20;temperature=kCold}:{age=10;temperature=kWarm}}")
+  //    {age=10;temperature=kWarm}:{age=20;temperature=kCold}}}")
   // In this example, all files that are at least 20 seconds old will be
   // compacted and output files will have temperature kCold. All files that are
   // at least 10 seconds old but younger than 20 seconds will be compacted to