From e7de808d35d155952677a58c54222f183bd87a6b Mon Sep 17 00:00:00 2001 From: myasuka Date: Thu, 24 Mar 2022 15:06:24 -0700 Subject: [PATCH] Enable READ_BLOCK_COMPACTION_MICROS to track stats (#9722) Summary: After commit [d642c60](https://github.com/facebook/rocksdb/commit/d642c60bdc100f7509ca77b383cd47b51d80d810), the stats `READ_BLOCK_COMPACTION_MICROS` cannot record any compaction read duration, and it always report zero. This PR targets to distinguish `READ_BLOCK_COMPACTION_MICROS` with `READ_BLOCK_GET_MICROS` so that `READ_BLOCK_COMPACTION_MICROS` could record the correct stats. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9722 Reviewed By: ajkr Differential Revision: D35021870 Pulled By: jay-zhuang fbshipit-source-id: f1a804994265e51465de64c2a08f2e0eeb6fc5a3 --- HISTORY.md | 1 + table/block_based/block_based_table_reader.cc | 23 +++++++++++-------- table/block_based/block_based_table_reader.h | 7 +++--- table/block_based/partitioned_filter_block.cc | 4 ++-- table/block_based/partitioned_index_reader.cc | 4 ++-- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 34f20138e..be8620c05 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -6,6 +6,7 @@ * Fixed a race condition when mmaping a WritableFile on POSIX. * Fixed a race condition when 2PC is disabled and WAL tracking in the MANIFEST is enabled. The race condition is between two background flush threads trying to install flush results, causing a WAL deletion not tracked in the MANIFEST. A future DB open may fail. * Fixed a heap use-after-free race with DropColumnFamily. +* Fixed a bug that `rocksdb.read.block.compaction.micros` cannot track compaction stats (#9722). ## 7.0.3 (03/23/2022) ### Bug Fixes diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index 388add9ed..f1f245f8a 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -1467,9 +1467,10 @@ template Status BlockBasedTable::MaybeReadBlockAndLoadToCache( FilePrefetchBuffer* prefetch_buffer, const ReadOptions& ro, const BlockHandle& handle, const UncompressionDict& uncompression_dict, - const bool wait, CachableEntry* block_entry, - BlockType block_type, GetContext* get_context, - BlockCacheLookupContext* lookup_context, BlockContents* contents) const { + const bool wait, const bool for_compaction, + CachableEntry* block_entry, BlockType block_type, + GetContext* get_context, BlockCacheLookupContext* lookup_context, + BlockContents* contents) const { assert(block_entry != nullptr); const bool no_io = (ro.read_tier == kBlockCacheTier); Cache* block_cache = rep_->table_options.block_cache.get(); @@ -1522,7 +1523,9 @@ Status BlockBasedTable::MaybeReadBlockAndLoadToCache( CompressionType raw_block_comp_type; BlockContents raw_block_contents; if (!contents) { - StopWatch sw(rep_->ioptions.clock, statistics, READ_BLOCK_GET_MICROS); + Histograms histogram = for_compaction ? READ_BLOCK_COMPACTION_MICROS + : READ_BLOCK_GET_MICROS; + StopWatch sw(rep_->ioptions.clock, statistics, histogram); BlockFetcher block_fetcher( rep_->file.get(), prefetch_buffer, rep_->footer, ro, handle, &raw_block_contents, rep_->ioptions, do_uncompress, @@ -1880,8 +1883,9 @@ void BlockBasedTable::RetrieveMultipleBlocks( // avoid looking up the block cache s = MaybeReadBlockAndLoadToCache( nullptr, options, handle, uncompression_dict, /*wait=*/true, - block_entry, BlockType::kData, mget_iter->get_context, - &lookup_data_block_context, &raw_block_contents); + /*for_compaction=*/false, block_entry, BlockType::kData, + mget_iter->get_context, &lookup_data_block_context, + &raw_block_contents); // block_entry value could be null if no block cache is present, i.e // BlockBasedTableOptions::no_block_cache is true and no compressed @@ -1935,7 +1939,7 @@ Status BlockBasedTable::RetrieveBlock( if (use_cache) { s = MaybeReadBlockAndLoadToCache( prefetch_buffer, ro, handle, uncompression_dict, wait_for_cache, - block_entry, block_type, get_context, lookup_context, + for_compaction, block_entry, block_type, get_context, lookup_context, /*contents=*/nullptr); if (!s.ok()) { @@ -1964,8 +1968,9 @@ Status BlockBasedTable::RetrieveBlock( std::unique_ptr block; { - StopWatch sw(rep_->ioptions.clock, rep_->ioptions.stats, - READ_BLOCK_GET_MICROS); + Histograms histogram = + for_compaction ? READ_BLOCK_COMPACTION_MICROS : READ_BLOCK_GET_MICROS; + StopWatch sw(rep_->ioptions.clock, rep_->ioptions.stats, histogram); s = ReadBlockFromFile( rep_->file.get(), prefetch_buffer, rep_->footer, ro, handle, &block, rep_->ioptions, do_uncompress, maybe_compressed, block_type, diff --git a/table/block_based/block_based_table_reader.h b/table/block_based/block_based_table_reader.h index 1163449da..eac4fd38c 100644 --- a/table/block_based/block_based_table_reader.h +++ b/table/block_based/block_based_table_reader.h @@ -343,9 +343,10 @@ class BlockBasedTable : public TableReader { Status MaybeReadBlockAndLoadToCache( FilePrefetchBuffer* prefetch_buffer, const ReadOptions& ro, const BlockHandle& handle, const UncompressionDict& uncompression_dict, - const bool wait, CachableEntry* block_entry, - BlockType block_type, GetContext* get_context, - BlockCacheLookupContext* lookup_context, BlockContents* contents) const; + const bool wait, const bool for_compaction, + CachableEntry* block_entry, BlockType block_type, + GetContext* get_context, BlockCacheLookupContext* lookup_context, + BlockContents* contents) const; // Similar to the above, with one crucial difference: it will retrieve the // block from the file even if there are no caches configured (assuming the diff --git a/table/block_based/partitioned_filter_block.cc b/table/block_based/partitioned_filter_block.cc index f9d53aba7..c33c383f1 100644 --- a/table/block_based/partitioned_filter_block.cc +++ b/table/block_based/partitioned_filter_block.cc @@ -519,8 +519,8 @@ Status PartitionedFilterBlockReader::CacheDependencies(const ReadOptions& ro, // filter blocks s = table()->MaybeReadBlockAndLoadToCache( prefetch_buffer.get(), ro, handle, UncompressionDict::GetEmptyDict(), - /* wait */ true, &block, BlockType::kFilter, nullptr /* get_context */, - &lookup_context, nullptr /* contents */); + /* wait */ true, /* for_compaction */ false, &block, BlockType::kFilter, + nullptr /* get_context */, &lookup_context, nullptr /* contents */); if (!s.ok()) { return s; } diff --git a/table/block_based/partitioned_index_reader.cc b/table/block_based/partitioned_index_reader.cc index e295d41a4..9ed418f02 100644 --- a/table/block_based/partitioned_index_reader.cc +++ b/table/block_based/partitioned_index_reader.cc @@ -180,8 +180,8 @@ Status PartitionIndexReader::CacheDependencies(const ReadOptions& ro, // filter blocks Status s = table()->MaybeReadBlockAndLoadToCache( prefetch_buffer.get(), ro, handle, UncompressionDict::GetEmptyDict(), - /*wait=*/true, &block, BlockType::kIndex, /*get_context=*/nullptr, - &lookup_context, /*contents=*/nullptr); + /*wait=*/true, /*for_compaction=*/false, &block, BlockType::kIndex, + /*get_context=*/nullptr, &lookup_context, /*contents=*/nullptr); if (!s.ok()) { return s;