diff --git a/db/compaction/compaction_picker_fifo.cc b/db/compaction/compaction_picker_fifo.cc index a3a2c660c2d..50529777028 100644 --- a/db/compaction/compaction_picker_fifo.cc +++ b/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, diff --git a/db/compaction/compaction_picker_fifo.h b/db/compaction/compaction_picker_fifo.h index 5b1b12d863b..df21a1bde0f 100644 --- a/db/compaction/compaction_picker_fifo.h +++ b/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 diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index 91206aa58c5..6231a4c27a1 100644 --- a/db/db_compaction_test.cc +++ b/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); } diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 10e8af97e20..d8365c0d0d2 100644 --- a/db/db_impl/db_impl.h +++ b/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& c); void BuildCompactionJobInfo(const ColumnFamilyData* cfd, Compaction* c, const Status& st, const CompactionJobStats& compaction_job_stats, diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index bdf4f389417..5af305d3102 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/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& 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, diff --git a/db/db_options_test.cc b/db/db_options_test.cc index 4c4c09a15df..f52982bbcbe 100644 --- a/db/db_options_test.cc +++ b/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() diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h index 2d3e52f8908..9d81e435f87 100644 --- a/include/rocksdb/statistics.h +++ b/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 }; diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h index bb0a2840712..03d17082818 100644 --- a/java/rocksjni/portal.h +++ b/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 diff --git a/java/src/main/java/org/rocksdb/TickerType.java b/java/src/main/java/org/rocksdb/TickerType.java index b63e5b8ce2a..a718dfa1515 100644 --- a/java/src/main/java/org/rocksdb/TickerType.java +++ b/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; diff --git a/monitoring/statistics.cc b/monitoring/statistics.cc index c4de1a0ce66..5aede1df173 100644 --- a/monitoring/statistics.cc +++ b/monitoring/statistics.cc @@ -261,8 +261,6 @@ const std::vector> 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> HistogramsNameMap = { diff --git a/unreleased_history/new_features/fifo_drop_file_new_stats.md b/unreleased_history/new_features/fifo_drop_file_new_stats.md index f6574915648..30134b2c2a8 100644 --- a/unreleased_history/new_features/fifo_drop_file_new_stats.md +++ b/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