Count completed deletion compactions only

db/compaction/compaction_picker_fifo.cc

@@ -446,32 +446,9 @@ Compaction* FIFOCompactionPicker::PickCompaction(
         cf_name, mutable_cf_options, mutable_db_options, vstorage, log_buffer);
   }
   RegisterCompaction(c);
-  UpdateCompactionStats(c);
   return c;
 }
 
-void FIFOCompactionPicker::UpdateCompactionStats(Compaction* c) {
-  if (c == nullptr || ioptions_.statistics == nullptr) {
-    return;
-  }
-
-  CompactionReason reason = c->compaction_reason();
-  Statistics* statistics = ioptions_.statistics.get();
-  switch (reason) {
-    case CompactionReason::kFIFOMaxSize:
-      RecordTick(statistics, FIFO_MAX_SIZE_COMPACTIONS);
-      break;
-    case CompactionReason::kFIFOTtl:
-      RecordTick(statistics, FIFO_TTL_COMPACTIONS);
-      break;
-    case CompactionReason::kChangeTemperature:
-      RecordTick(statistics, FIFO_CHANGE_TEMPERATURE_COMPACTIONS);
-      break;
-    default:
-      break;
-  }
-}
-
 Compaction* FIFOCompactionPicker::CompactRange(
     const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
     const MutableDBOptions& mutable_db_options, VersionStorageInfo* vstorage,

db/compaction/compaction_picker_fifo.h

@@ -56,7 +56,5 @@ class FIFOCompactionPicker : public CompactionPicker {
       const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
       const MutableDBOptions& mutable_db_options, VersionStorageInfo* vstorage,
       LogBuffer* log_buffer);
-
-  void UpdateCompactionStats(Compaction* c);
 };
 }  // namespace ROCKSDB_NAMESPACE

db/db_compaction_test.cc

@@ -9007,7 +9007,6 @@ TEST_F(DBCompactionTest, FIFOChangeTemperature) {
   options.max_open_files = -1;
   options.level0_file_num_compaction_trigger = 2;
   options.create_if_missing = true;
-  options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
   CompactionOptionsFIFO fifo_options;
   fifo_options.file_temperature_age_thresholds = {{Temperature::kCold, 1000}};
   fifo_options.max_table_files_size = 100000000;
@@ -9061,12 +9060,6 @@ TEST_F(DBCompactionTest, FIFOChangeTemperature) {
   ASSERT_EQ(Temperature::kCold, metadata.levels[0].files[3].temperature);
   ASSERT_EQ(2, total_cold);
 
-  ASSERT_GT(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
-  ASSERT_EQ(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
-  ASSERT_EQ(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
-
   Destroy(options);
 }
 

db/db_impl/db_impl.h

@@ -2248,6 +2248,7 @@ class DBImpl : public DB {
   bool ShouldntRunManualCompaction(ManualCompactionState* m);
   bool HaveManualCompaction(ColumnFamilyData* cfd);
   bool MCOverlap(ManualCompactionState* m, ManualCompactionState* m1);
+  void UpdateDeletionCompactionStats(const std::unique_ptr<Compaction>& c);
   void BuildCompactionJobInfo(const ColumnFamilyData* cfd, Compaction* c,
                               const Status& st,
                               const CompactionJobStats& compaction_job_stats,

db/db_impl/db_impl_compaction_flush.cc

@@ -3704,6 +3704,9 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
     ROCKS_LOG_BUFFER(log_buffer, "[%s] Deleted %d files\n",
                      c->column_family_data()->GetName().c_str(),
                      c->num_input_files(0));
+    if (status.ok() && io_s.ok()) {
+      UpdateDeletionCompactionStats(c);
+    }
     *made_progress = true;
     TEST_SYNC_POINT_CALLBACK("DBImpl::BackgroundCompaction:AfterCompaction",
                              c->column_family_data());
@@ -4082,6 +4085,27 @@ bool DBImpl::MCOverlap(ManualCompactionState* m, ManualCompactionState* m1) {
   return false;
 }
 
+void DBImpl::UpdateDeletionCompactionStats(
+    const std::unique_ptr<Compaction>& c) {
+  if (c == nullptr) {
+    return;
+  }
+
+  CompactionReason reason = c->compaction_reason();
+
+  switch (reason) {
+    case CompactionReason::kFIFOMaxSize:
+      RecordTick(stats_, FIFO_MAX_SIZE_COMPACTIONS);
+      break;
+    case CompactionReason::kFIFOTtl:
+      RecordTick(stats_, FIFO_TTL_COMPACTIONS);
+      break;
+    default:
+      assert(false);
+      break;
+  }
+}
+
 void DBImpl::BuildCompactionJobInfo(
     const ColumnFamilyData* cfd, Compaction* c, const Status& st,
     const CompactionJobStats& compaction_job_stats, const int job_id,

db/db_options_test.cc

@@ -994,9 +994,6 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
   ASSERT_EQ(NumTableFilesAtLevel(0), 10);
 
   ASSERT_EQ(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
-  ASSERT_EQ(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
   ASSERT_EQ(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
 
   // Set ttl to 1 minute. So all files should get deleted.
@@ -1007,9 +1004,6 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
   ASSERT_EQ(NumTableFilesAtLevel(0), 0);
 
   ASSERT_GT(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
-  ASSERT_EQ(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
   ASSERT_EQ(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
   ASSERT_OK(options.statistics->Reset());
 
@@ -1037,9 +1031,6 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
   ASSERT_EQ(NumTableFilesAtLevel(0), 10);
 
   ASSERT_EQ(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
-  ASSERT_EQ(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
   ASSERT_EQ(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
 
   // Set max_table_files_size to 12 KB. So only 1 file should remain now.
@@ -1052,9 +1043,6 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
   ASSERT_EQ(NumTableFilesAtLevel(0), 1);
 
   ASSERT_GT(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
-  ASSERT_EQ(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
   ASSERT_EQ(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
   ASSERT_OK(options.statistics->Reset());
 
@@ -1082,13 +1070,6 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
   ASSERT_OK(dbfull()->CompactRange(CompactRangeOptions(), nullptr, nullptr));
   ASSERT_EQ(NumTableFilesAtLevel(0), 10);
 
-  ASSERT_EQ(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
-  ASSERT_EQ(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
-  ASSERT_EQ(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
-  ASSERT_OK(options.statistics->Reset());
-
   // Set allow_compaction to true. So number of files should be between 1 and 5.
   ASSERT_OK(dbfull()->SetOptions(
       {{"compaction_options_fifo", "{allow_compaction=true;}"}}));
@@ -1099,13 +1080,6 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
   ASSERT_GE(NumTableFilesAtLevel(0), 1);
   ASSERT_LE(NumTableFilesAtLevel(0), 5);
 
-  ASSERT_EQ(options.statistics->getTickerCount(FIFO_MAX_SIZE_COMPACTIONS), 0);
-  ASSERT_EQ(
-      options.statistics->getTickerCount(FIFO_CHANGE_TEMPERATURE_COMPACTIONS),
-      0);
-  ASSERT_EQ(options.statistics->getTickerCount(FIFO_TTL_COMPACTIONS), 0);
-  ASSERT_OK(options.statistics->Reset());
-
   // Test dynamically setting `file_temperature_age_thresholds`
   ASSERT_TRUE(
       dbfull()

include/rocksdb/statistics.h

@@ -521,7 +521,6 @@ enum Tickers : uint32_t {
   // Number of FIFO compactions that drop files based on different reasons
   FIFO_MAX_SIZE_COMPACTIONS,
   FIFO_TTL_COMPACTIONS,
-  FIFO_CHANGE_TEMPERATURE_COMPACTIONS,
 
   TICKER_ENUM_MAX
 };

java/rocksjni/portal.h

@@ -5169,8 +5169,6 @@ class TickerTypeJni {
         return -0x3E;
       case ROCKSDB_NAMESPACE::Tickers::FIFO_TTL_COMPACTIONS:
         return -0x3F;
-      case ROCKSDB_NAMESPACE::Tickers::FIFO_CHANGE_TEMPERATURE_COMPACTIONS:
-        return -0x40;
       case ROCKSDB_NAMESPACE::Tickers::TICKER_ENUM_MAX:
         // 0x5F was the max value in the initial copy of tickers to Java.
         // Since these values are exposed directly to Java clients, we keep
@@ -5540,8 +5538,6 @@ class TickerTypeJni {
         return ROCKSDB_NAMESPACE::Tickers::FIFO_MAX_SIZE_COMPACTIONS;
       case -0x3F:
         return ROCKSDB_NAMESPACE::Tickers::FIFO_TTL_COMPACTIONS;
-      case -0x40:
-        return ROCKSDB_NAMESPACE::Tickers::FIFO_CHANGE_TEMPERATURE_COMPACTIONS;
       case 0x5F:
         // 0x5F was the max value in the initial copy of tickers to Java.
         // Since these values are exposed directly to Java clients, we keep

java/src/main/java/org/rocksdb/TickerType.java

@@ -770,8 +770,6 @@ public enum TickerType {
 
     FIFO_TTL_COMPACTIONS((byte) -0x3F),
 
-    FIFO_CHANGE_TEMPERATURE_COMPACTIONS((byte) -0x40),
-
     TICKER_ENUM_MAX((byte) 0x5F);
 
     private final byte value;

monitoring/statistics.cc

@@ -261,8 +261,6 @@ const std::vector<std::pair<Tickers, std::string>> TickersNameMap = {
     {READAHEAD_TRIMMED, "rocksdb.readahead.trimmed"},
     {FIFO_MAX_SIZE_COMPACTIONS, "rocksdb.fifo.max.size.compactions"},
     {FIFO_TTL_COMPACTIONS, "rocksdb.fifo.ttl.compactions"},
-    {FIFO_CHANGE_TEMPERATURE_COMPACTIONS,
-     "rocksdb.fifo.change.temperature.compactions"},
 };
 
 const std::vector<std::pair<Histograms, std::string>> HistogramsNameMap = {

unreleased_history/new_features/fifo_drop_file_new_stats.md

@@ -1 +1 @@
-Added new tickers `rocksdb.fifo.{max.size|ttl|change.temperature}.compactions` to count FIFO compactions that drop files for different reasons
+Added new tickers `rocksdb.fifo.{max.size|ttl}.compactions` to count FIFO compactions that drop files for different reasons