From 0e7b27bfcf82858d78512c822c641175d12dd215 Mon Sep 17 00:00:00 2001 From: akankshamahajan Date: Fri, 21 Oct 2022 12:15:35 -0700 Subject: [PATCH] Refactor block cache tracing APIs (#10811) Summary: Refactor the classes, APIs and data structures for block cache tracing to allow a user provided trace writer to be used. Currently, only a TraceWriter is supported, with a default built-in implementation of FileTraceWriter. The TraceWriter, however, takes a flat trace record and is thus only suitable for file tracing. This PR introduces an abstract BlockCacheTraceWriter class that takes a structured BlockCacheTraceRecord. The BlockCacheTraceWriter implementation can then format and log the record in whatever way it sees fit. The default BlockCacheTraceWriterImpl does file tracing using a user provided TraceWriter. `DB::StartBlockTrace` will internally redirect to changed `BlockCacheTrace::StartBlockCacheTrace`. New API `DB::StartBlockTrace` is also added that directly takes `BlockCacheTraceWriter` pointer. This same philosophy can be applied to KV and IO tracing as well. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10811 Test Plan: existing unit tests Old API DB::StartBlockTrace checked with db_bench tool create database ``` ./db_bench --benchmarks="fillseq" \ --key_size=20 --prefix_size=20 --keys_per_prefix=0 --value_size=100 \ --cache_index_and_filter_blocks --cache_size=1048576 \ --disable_auto_compactions=1 --disable_wal=1 --compression_type=none \ --min_level_to_compress=-1 --compression_ratio=1 --num=10000000 ``` To trace block cache accesses when running readrandom benchmark: ``` ./db_bench --benchmarks="readrandom" --use_existing_db --duration=60 \ --key_size=20 --prefix_size=20 --keys_per_prefix=0 --value_size=100 \ --cache_index_and_filter_blocks --cache_size=1048576 \ --disable_auto_compactions=1 --disable_wal=1 --compression_type=none \ --min_level_to_compress=-1 --compression_ratio=1 --num=10000000 \ --threads=16 \ -block_cache_trace_file="/tmp/binary_trace_test_example" \ -block_cache_trace_max_trace_file_size_in_bytes=1073741824 \ -block_cache_trace_sampling_frequency=1 ``` Reviewed By: anand1976 Differential Revision: D40435289 Pulled By: akankshamahajan15 fbshipit-source-id: fa2755f4788185e19f4605e731641cfd21ab3282 --- HISTORY.md | 3 + db/db_impl/db_impl.cc | 20 ++- db/db_impl/db_impl.h | 6 +- include/rocksdb/block_cache_trace_writer.h | 149 ++++++++++++++++++ include/rocksdb/db.h | 9 +- .../rocksdb}/table_reader_caller.h | 0 include/rocksdb/utilities/stackable_db.h | 8 +- table/table_reader.h | 2 +- table/table_test.cc | 48 +++--- .../block_cache_trace_analyzer.cc | 2 +- .../block_cache_trace_analyzer.h | 2 +- .../block_cache_trace_analyzer_test.cc | 41 +++-- trace_replay/block_cache_tracer.cc | 49 +++--- trace_replay/block_cache_tracer.h | 88 ++--------- trace_replay/block_cache_tracer_test.cc | 112 ++++++++----- utilities/simulator_cache/cache_simulator.cc | 4 +- .../simulator_cache/cache_simulator_test.cc | 26 +-- 17 files changed, 378 insertions(+), 191 deletions(-) create mode 100644 include/rocksdb/block_cache_trace_writer.h rename {table => include/rocksdb}/table_reader_caller.h (100%) diff --git a/HISTORY.md b/HISTORY.md index a959c26be..c6f1f58ce 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -32,6 +32,9 @@ ### Behavior Changes * Sanitize min_write_buffer_number_to_merge to 1 if atomic flush is enabled to prevent unexpected data loss when WAL is disabled in a multi-column-family setting (#10773). +### Public API changes +* Refactor the classes, APIs and data structures for block cache tracing to allow a user provided trace writer to be used. Introduced an abstract BlockCacheTraceWriter class that takes a structured BlockCacheTraceRecord. The BlockCacheTraceWriter implementation can then format and log the record in whatever way it sees fit. The default BlockCacheTraceWriterImpl does file tracing using a user provided TraceWriter. More details in rocksdb/includb/block_cache_trace_writer.h. + ## 7.7.0 (09/18/2022) ### Bug Fixes * Fixed a hang when an operation such as `GetLiveFiles` or `CreateNewBackup` is asked to trigger and wait for memtable flush on a read-only DB. Such indirect requests for memtable flush are now ignored on a read-only DB. diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index d0b6f085b..eb870fd65 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -5782,8 +5782,24 @@ Status DBImpl::NewDefaultReplayer( Status DBImpl::StartBlockCacheTrace( const TraceOptions& trace_options, std::unique_ptr&& trace_writer) { - return block_cache_tracer_.StartTrace(immutable_db_options_.clock, - trace_options, std::move(trace_writer)); + BlockCacheTraceOptions block_trace_opts; + block_trace_opts.sampling_frequency = trace_options.sampling_frequency; + + BlockCacheTraceWriterOptions trace_writer_opt; + trace_writer_opt.max_trace_file_size = trace_options.max_trace_file_size; + + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + + return block_cache_tracer_.StartTrace(block_trace_opts, + std::move(block_cache_trace_writer)); +} + +Status DBImpl::StartBlockCacheTrace( + const BlockCacheTraceOptions& trace_options, + std::unique_ptr&& trace_writer) { + return block_cache_tracer_.StartTrace(trace_options, std::move(trace_writer)); } Status DBImpl::EndBlockCacheTrace() { diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index dfc701b30..f9aef72f6 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -568,9 +568,13 @@ class DBImpl : public DB { using DB::StartBlockCacheTrace; Status StartBlockCacheTrace( - const TraceOptions& options, + const TraceOptions& trace_options, std::unique_ptr&& trace_writer) override; + Status StartBlockCacheTrace( + const BlockCacheTraceOptions& options, + std::unique_ptr&& trace_writer) override; + using DB::EndBlockCacheTrace; Status EndBlockCacheTrace() override; diff --git a/include/rocksdb/block_cache_trace_writer.h b/include/rocksdb/block_cache_trace_writer.h new file mode 100644 index 000000000..d5047bbc7 --- /dev/null +++ b/include/rocksdb/block_cache_trace_writer.h @@ -0,0 +1,149 @@ +// Copyright (c) 2022, Meta, Inc. All rights reserved. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). + +#pragma once + +#include "rocksdb/options.h" +#include "rocksdb/system_clock.h" +#include "rocksdb/table_reader_caller.h" +#include "rocksdb/trace_reader_writer.h" +#include "rocksdb/trace_record.h" + +namespace ROCKSDB_NAMESPACE { +// A record for block cache lookups/inserts. This is passed by the table +// reader to the BlockCacheTraceWriter for every block cache op. +struct BlockCacheTraceRecord { + // Required fields for all accesses. + uint64_t access_timestamp = 0; + + // Info related to the block being looked up or inserted + // + // 1. The cache key for the block + std::string block_key; + + // 2. The type of block + TraceType block_type = TraceType::kTraceMax; + + // 3. Size of the block + uint64_t block_size = 0; + + // Info about the SST file the block is in + // + // 1. Column family ID + uint64_t cf_id = 0; + + // 2. Column family name + std::string cf_name; + + // 3. LSM level of the file + uint32_t level = 0; + + // 4. SST file number + uint64_t sst_fd_number = 0; + + // Info about the calling context + // + // 1. The higher level request triggering the block cache request + TableReaderCaller caller = TableReaderCaller::kMaxBlockCacheLookupCaller; + + // 2. Cache lookup hit/miss. Not relevant for inserts + bool is_cache_hit = false; + + // 3. Whether this request is a lookup + bool no_insert = false; + + // Get/MultiGet specific info + // + // 1. A unique ID for Get/MultiGet + uint64_t get_id = kReservedGetId; + + // 2. Whether the Get/MultiGet is from a user-specified snapshot + bool get_from_user_specified_snapshot = false; + + // 3. The target user key in the block + std::string referenced_key; + + // Required fields for data block and user Get/Multi-Get only. + // + // 1. Size of te useful data in the block + uint64_t referenced_data_size = 0; + + // 2. Only for MultiGet, number of keys from the batch found in the block + uint64_t num_keys_in_block = 0; + + // 3. Whether the key was found in the block or not (false positive) + bool referenced_key_exist_in_block = false; + + static const uint64_t kReservedGetId; + + BlockCacheTraceRecord() {} + + BlockCacheTraceRecord(uint64_t _access_timestamp, std::string _block_key, + TraceType _block_type, uint64_t _block_size, + uint64_t _cf_id, std::string _cf_name, uint32_t _level, + uint64_t _sst_fd_number, TableReaderCaller _caller, + bool _is_cache_hit, bool _no_insert, uint64_t _get_id, + bool _get_from_user_specified_snapshot = false, + std::string _referenced_key = "", + uint64_t _referenced_data_size = 0, + uint64_t _num_keys_in_block = 0, + bool _referenced_key_exist_in_block = false) + : access_timestamp(_access_timestamp), + block_key(_block_key), + block_type(_block_type), + block_size(_block_size), + cf_id(_cf_id), + cf_name(_cf_name), + level(_level), + sst_fd_number(_sst_fd_number), + caller(_caller), + is_cache_hit(_is_cache_hit), + no_insert(_no_insert), + get_id(_get_id), + get_from_user_specified_snapshot(_get_from_user_specified_snapshot), + referenced_key(_referenced_key), + referenced_data_size(_referenced_data_size), + num_keys_in_block(_num_keys_in_block), + referenced_key_exist_in_block(_referenced_key_exist_in_block) {} +}; + +// Options for tracing block cache accesses +struct BlockCacheTraceOptions { + // Specify trace sampling option, i.e. capture one per how many requests. + // Default to 1 (capture every request). + uint64_t sampling_frequency = 1; +}; + +// Options for the built-in implementation of BlockCacheTraceWriter +struct BlockCacheTraceWriterOptions { + uint64_t max_trace_file_size = uint64_t{64} * 1024 * 1024 * 1024; +}; + +// BlockCacheTraceWriter is an abstract class that captures all RocksDB block +// cache accesses. Every RocksDB operation is passed to WriteBlockAccess() +// with a BlockCacheTraceRecord. +class BlockCacheTraceWriter { + public: + virtual ~BlockCacheTraceWriter() {} + + // Pass Slice references to avoid copy. + virtual Status WriteBlockAccess(const BlockCacheTraceRecord& record, + const Slice& block_key, const Slice& cf_name, + const Slice& referenced_key) = 0; + + // Write a trace header at the beginning, typically on initiating a trace, + // with some metadata like a magic number and RocksDB version. + virtual Status WriteHeader() = 0; +}; + +// Allocate an instance of the built-in BlockCacheTraceWriter implementation, +// that traces all block cache accesses to a user-provided TraceWriter. Each +// access is traced to a file with a timestamp and type, followed by the +// payload. +std::unique_ptr NewBlockCacheTraceWriter( + SystemClock* clock, const BlockCacheTraceWriterOptions& trace_options, + std::unique_ptr&& trace_writer); + +} // namespace ROCKSDB_NAMESPACE diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index a16db3b54..fc23a4995 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -17,6 +17,7 @@ #include #include +#include "rocksdb/block_cache_trace_writer.h" #include "rocksdb/iterator.h" #include "rocksdb/listener.h" #include "rocksdb/metadata.h" @@ -1748,11 +1749,17 @@ class DB { // Trace block cache accesses. Use EndBlockCacheTrace() to stop tracing. virtual Status StartBlockCacheTrace( - const TraceOptions& /*options*/, + const TraceOptions& /*trace_options*/, std::unique_ptr&& /*trace_writer*/) { return Status::NotSupported("StartBlockCacheTrace() is not implemented."); } + virtual Status StartBlockCacheTrace( + const BlockCacheTraceOptions& /*options*/, + std::unique_ptr&& /*trace_writer*/) { + return Status::NotSupported("StartBlockCacheTrace() is not implemented."); + } + virtual Status EndBlockCacheTrace() { return Status::NotSupported("EndBlockCacheTrace() is not implemented."); } diff --git a/table/table_reader_caller.h b/include/rocksdb/table_reader_caller.h similarity index 100% rename from table/table_reader_caller.h rename to include/rocksdb/table_reader_caller.h diff --git a/include/rocksdb/utilities/stackable_db.h b/include/rocksdb/utilities/stackable_db.h index a0281400f..09e3b0249 100644 --- a/include/rocksdb/utilities/stackable_db.h +++ b/include/rocksdb/utilities/stackable_db.h @@ -404,8 +404,14 @@ class StackableDB : public DB { using DB::StartBlockCacheTrace; Status StartBlockCacheTrace( - const TraceOptions& options, + const TraceOptions& trace_options, std::unique_ptr&& trace_writer) override { + return db_->StartBlockCacheTrace(trace_options, std::move(trace_writer)); + } + + Status StartBlockCacheTrace( + const BlockCacheTraceOptions& options, + std::unique_ptr&& trace_writer) override { return db_->StartBlockCacheTrace(options, std::move(trace_writer)); } diff --git a/table/table_reader.h b/table/table_reader.h index 5374edf7c..4a904991d 100644 --- a/table/table_reader.h +++ b/table/table_reader.h @@ -15,10 +15,10 @@ #include "folly/experimental/coro/Task.h" #endif #include "rocksdb/slice_transform.h" +#include "rocksdb/table_reader_caller.h" #include "table/get_context.h" #include "table/internal_iterator.h" #include "table/multiget_context.h" -#include "table/table_reader_caller.h" namespace ROCKSDB_NAMESPACE { diff --git a/table/table_test.cc b/table/table_test.cc index a2d96c6d0..5759a4515 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -1147,15 +1147,21 @@ class BlockBasedTableTest test_path_ = test::PerThreadDBPath("block_based_table_tracing_test"); EXPECT_OK(env_->CreateDir(test_path_)); trace_file_path_ = test_path_ + "/block_cache_trace_file"; - TraceOptions trace_opt; + + BlockCacheTraceWriterOptions trace_writer_opt; + BlockCacheTraceOptions trace_opt; std::unique_ptr trace_writer; EXPECT_OK(NewFileTraceWriter(env_, EnvOptions(), trace_file_path_, &trace_writer)); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); // Always return Status::OK(). assert(c->block_cache_tracer_ - .StartTrace(env_->GetSystemClock().get(), trace_opt, - std::move(trace_writer)) + .StartTrace(trace_opt, std::move(block_cache_trace_writer)) .ok()); + { std::string user_key = "k01"; InternalKey internal_key(user_key, 0, kTypeValue); @@ -1213,10 +1219,10 @@ class BlockBasedTableTest } else { EXPECT_EQ(access.referenced_key, ""); EXPECT_EQ(access.get_id, 0); - EXPECT_TRUE(access.get_from_user_specified_snapshot == Boolean::kFalse); + EXPECT_FALSE(access.get_from_user_specified_snapshot); EXPECT_EQ(access.referenced_data_size, 0); EXPECT_EQ(access.num_keys_in_block, 0); - EXPECT_TRUE(access.referenced_key_exist_in_block == Boolean::kFalse); + EXPECT_FALSE(access.referenced_key_exist_in_block); } index++; } @@ -3051,8 +3057,8 @@ TEST_P(BlockBasedTableTest, TracingGetTest) { BlockCacheTraceRecord record; record.block_type = TraceType::kBlockTraceIndexBlock; record.caller = TableReaderCaller::kPrefetch; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceFilterBlock; expected_records.push_back(record); @@ -3061,22 +3067,22 @@ TEST_P(BlockBasedTableTest, TracingGetTest) { record.get_id = 1; record.block_type = TraceType::kBlockTraceFilterBlock; record.caller = TableReaderCaller::kUserGet; - record.get_from_user_specified_snapshot = Boolean::kFalse; + record.get_from_user_specified_snapshot = false; record.referenced_key = encoded_key; - record.referenced_key_exist_in_block = Boolean::kTrue; - record.is_cache_hit = Boolean::kTrue; + record.referenced_key_exist_in_block = true; + record.is_cache_hit = true; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceIndexBlock; expected_records.push_back(record); - record.is_cache_hit = Boolean::kFalse; + record.is_cache_hit = false; record.block_type = TraceType::kBlockTraceDataBlock; expected_records.push_back(record); // The second get should all observe cache hits. - record.is_cache_hit = Boolean::kTrue; + record.is_cache_hit = true; record.get_id = 2; record.block_type = TraceType::kBlockTraceFilterBlock; record.caller = TableReaderCaller::kUserGet; - record.get_from_user_specified_snapshot = Boolean::kFalse; + record.get_from_user_specified_snapshot = false; record.referenced_key = encoded_key; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceIndexBlock; @@ -3116,15 +3122,15 @@ TEST_P(BlockBasedTableTest, TracingApproximateOffsetOfTest) { BlockCacheTraceRecord record; record.block_type = TraceType::kBlockTraceIndexBlock; record.caller = TableReaderCaller::kPrefetch; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceFilterBlock; expected_records.push_back(record); // Then we should have two records for only index blocks. record.block_type = TraceType::kBlockTraceIndexBlock; record.caller = TableReaderCaller::kUserApproximateSize; - record.is_cache_hit = Boolean::kTrue; + record.is_cache_hit = true; expected_records.push_back(record); expected_records.push_back(record); VerifyBlockAccessTrace(&c, expected_records); @@ -3169,24 +3175,24 @@ TEST_P(BlockBasedTableTest, TracingIterator) { BlockCacheTraceRecord record; record.block_type = TraceType::kBlockTraceIndexBlock; record.caller = TableReaderCaller::kPrefetch; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceFilterBlock; expected_records.push_back(record); // Then we should have three records for index and two data block access. record.block_type = TraceType::kBlockTraceIndexBlock; record.caller = TableReaderCaller::kUserIterator; - record.is_cache_hit = Boolean::kTrue; + record.is_cache_hit = true; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceDataBlock; - record.is_cache_hit = Boolean::kFalse; + record.is_cache_hit = false; expected_records.push_back(record); expected_records.push_back(record); // When we iterate this file for the second time, we should observe all cache // hits. record.block_type = TraceType::kBlockTraceIndexBlock; - record.is_cache_hit = Boolean::kTrue; + record.is_cache_hit = true; expected_records.push_back(record); record.block_type = TraceType::kBlockTraceDataBlock; expected_records.push_back(record); diff --git a/tools/block_cache_analyzer/block_cache_trace_analyzer.cc b/tools/block_cache_analyzer/block_cache_trace_analyzer.cc index 59ad7004b..963719e95 100644 --- a/tools/block_cache_analyzer/block_cache_trace_analyzer.cc +++ b/tools/block_cache_analyzer/block_cache_trace_analyzer.cc @@ -1568,7 +1568,7 @@ Status BlockCacheTraceAnalyzer::Analyze() { trace_end_timestamp_in_seconds_ = access.access_timestamp / kMicrosInSecond; miss_ratio_stats_.UpdateMetrics(access.access_timestamp, is_user_access(access.caller), - access.is_cache_hit == Boolean::kFalse); + !access.is_cache_hit); if (cache_simulator_) { cache_simulator_->Access(access); } diff --git a/tools/block_cache_analyzer/block_cache_trace_analyzer.h b/tools/block_cache_analyzer/block_cache_trace_analyzer.h index 8afdf5449..e5bc3da31 100644 --- a/tools/block_cache_analyzer/block_cache_trace_analyzer.h +++ b/tools/block_cache_analyzer/block_cache_trace_analyzer.h @@ -95,7 +95,7 @@ struct BlockAccessInfo { if (BlockCacheTraceHelper::IsGetOrMultiGetOnDataBlock(access.block_type, access.caller)) { num_keys = access.num_keys_in_block; - if (access.referenced_key_exist_in_block == Boolean::kTrue) { + if (access.referenced_key_exist_in_block) { if (key_num_access_map.find(access.referenced_key) == key_num_access_map.end()) { referenced_data_size += access.referenced_data_size; diff --git a/tools/block_cache_analyzer/block_cache_trace_analyzer_test.cc b/tools/block_cache_analyzer/block_cache_trace_analyzer_test.cc index 915ad6117..c5d9b1452 100644 --- a/tools/block_cache_analyzer/block_cache_trace_analyzer_test.cc +++ b/tools/block_cache_analyzer/block_cache_trace_analyzer_test.cc @@ -114,14 +114,14 @@ class BlockCacheTracerTest : public testing::Test { } else { record.sst_fd_number = kSSTStoringOddKeys; } - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; // Provide these fields for all block types. // The writer should only write these fields for data blocks and the // caller is either GET or MGET. record.referenced_key = kRefKeyPrefix + std::to_string(key_id) + std::string(8, 0); - record.referenced_key_exist_in_block = Boolean::kTrue; + record.referenced_key_exist_in_block = true; record.num_keys_in_block = kNumKeysInBlock; ASSERT_OK(writer->WriteBlockAccess( record, record.block_key, record.cf_name, record.referenced_key)); @@ -223,15 +223,18 @@ class BlockCacheTracerTest : public testing::Test { TEST_F(BlockCacheTracerTest, BlockCacheAnalyzer) { { // Generate a trace file. - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; std::unique_ptr trace_writer; ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); const auto& clock = env_->GetSystemClock(); - BlockCacheTraceWriter writer(clock.get(), trace_opt, + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(clock.get(), trace_writer_opt, std::move(trace_writer)); - ASSERT_OK(writer.WriteHeader()); - WriteBlockAccess(&writer, 0, TraceType::kBlockTraceDataBlock, 50); + ASSERT_NE(block_cache_trace_writer, nullptr); + ASSERT_OK(block_cache_trace_writer->WriteHeader()); + WriteBlockAccess(block_cache_trace_writer.get(), 0, + TraceType::kBlockTraceDataBlock, 50); ASSERT_OK(env_->FileExists(trace_file_path_)); } { @@ -612,21 +615,27 @@ TEST_F(BlockCacheTracerTest, MixedBlocks) { // It contains two SST files with 25 blocks of odd numbered block_key in // kSSTStoringOddKeys and 25 blocks of even numbered blocks_key in // kSSTStoringEvenKeys. - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; std::unique_ptr trace_writer; const auto& clock = env_->GetSystemClock(); ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); - BlockCacheTraceWriter writer(clock.get(), trace_opt, + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(clock.get(), trace_writer_opt, std::move(trace_writer)); - ASSERT_OK(writer.WriteHeader()); + ASSERT_NE(block_cache_trace_writer, nullptr); + ASSERT_OK(block_cache_trace_writer->WriteHeader()); // Write blocks of different types. - WriteBlockAccess(&writer, 0, TraceType::kBlockTraceUncompressionDictBlock, - 10); - WriteBlockAccess(&writer, 10, TraceType::kBlockTraceDataBlock, 10); - WriteBlockAccess(&writer, 20, TraceType::kBlockTraceFilterBlock, 10); - WriteBlockAccess(&writer, 30, TraceType::kBlockTraceIndexBlock, 10); - WriteBlockAccess(&writer, 40, TraceType::kBlockTraceRangeDeletionBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 0, + TraceType::kBlockTraceUncompressionDictBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 10, + TraceType::kBlockTraceDataBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 20, + TraceType::kBlockTraceFilterBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 30, + TraceType::kBlockTraceIndexBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 40, + TraceType::kBlockTraceRangeDeletionBlock, 10); ASSERT_OK(env_->FileExists(trace_file_path_)); } diff --git a/trace_replay/block_cache_tracer.cc b/trace_replay/block_cache_tracer.cc index 4bd18cda7..508596913 100644 --- a/trace_replay/block_cache_tracer.cc +++ b/trace_replay/block_cache_tracer.cc @@ -20,7 +20,8 @@ namespace ROCKSDB_NAMESPACE { namespace { -bool ShouldTrace(const Slice& block_key, const TraceOptions& trace_options) { +bool ShouldTrace(const Slice& block_key, + const BlockCacheTraceOptions& trace_options) { if (trace_options.sampling_frequency == 0 || trace_options.sampling_frequency == 1) { return true; @@ -36,6 +37,7 @@ const uint64_t kSecondInMinute = 60; const uint64_t kSecondInHour = 3600; const std::string BlockCacheTraceHelper::kUnknownColumnFamilyName = "UnknownColumnFamily"; +const uint64_t BlockCacheTraceRecord::kReservedGetId = 0; const uint64_t BlockCacheTraceHelper::kReservedGetId = 0; bool BlockCacheTraceHelper::IsGetOrMultiGetOnDataBlock( @@ -79,9 +81,9 @@ uint64_t BlockCacheTraceHelper::GetSequenceNumber( if (!IsGetOrMultiGet(access.caller)) { return 0; } - return access.get_from_user_specified_snapshot == Boolean::kFalse - ? 0 - : 1 + GetInternalKeySeqno(access.referenced_key); + return access.get_from_user_specified_snapshot + ? 1 + GetInternalKeySeqno(access.referenced_key) + : 0; } uint64_t BlockCacheTraceHelper::GetBlockOffsetInFile( @@ -99,14 +101,14 @@ uint64_t BlockCacheTraceHelper::GetBlockOffsetInFile( return offset; } -BlockCacheTraceWriter::BlockCacheTraceWriter( - SystemClock* clock, const TraceOptions& trace_options, +BlockCacheTraceWriterImpl::BlockCacheTraceWriterImpl( + SystemClock* clock, const BlockCacheTraceWriterOptions& trace_options, std::unique_ptr&& trace_writer) : clock_(clock), trace_options_(trace_options), trace_writer_(std::move(trace_writer)) {} -Status BlockCacheTraceWriter::WriteBlockAccess( +Status BlockCacheTraceWriterImpl::WriteBlockAccess( const BlockCacheTraceRecord& record, const Slice& block_key, const Slice& cf_name, const Slice& referenced_key) { uint64_t trace_file_size = trace_writer_->GetFileSize(); @@ -141,7 +143,7 @@ Status BlockCacheTraceWriter::WriteBlockAccess( return trace_writer_->Write(encoded_trace); } -Status BlockCacheTraceWriter::WriteHeader() { +Status BlockCacheTraceWriterImpl::WriteHeader() { Trace trace; trace.ts = clock_->NowMicros(); trace.type = TraceType::kTraceBegin; @@ -255,13 +257,13 @@ Status BlockCacheTraceReader::ReadAccess(BlockCacheTraceRecord* record) { return Status::Incomplete( "Incomplete access record: Failed to read is_cache_hit."); } - record->is_cache_hit = static_cast(enc_slice[0]); + record->is_cache_hit = static_cast(enc_slice[0]); enc_slice.remove_prefix(kCharSize); if (enc_slice.empty()) { return Status::Incomplete( "Incomplete access record: Failed to read no_insert."); } - record->no_insert = static_cast(enc_slice[0]); + record->no_insert = static_cast(enc_slice[0]); enc_slice.remove_prefix(kCharSize); if (BlockCacheTraceHelper::IsGetOrMultiGet(record->caller)) { if (!GetFixed64(&enc_slice, &record->get_id)) { @@ -273,8 +275,7 @@ Status BlockCacheTraceReader::ReadAccess(BlockCacheTraceRecord* record) { "Incomplete access record: Failed to read " "get_from_user_specified_snapshot."); } - record->get_from_user_specified_snapshot = - static_cast(enc_slice[0]); + record->get_from_user_specified_snapshot = static_cast(enc_slice[0]); enc_slice.remove_prefix(kCharSize); Slice referenced_key; if (!GetLengthPrefixedSlice(&enc_slice, &referenced_key)) { @@ -299,7 +300,7 @@ Status BlockCacheTraceReader::ReadAccess(BlockCacheTraceRecord* record) { "Incomplete access record: Failed to read " "referenced_key_exist_in_block."); } - record->referenced_key_exist_in_block = static_cast(enc_slice[0]); + record->referenced_key_exist_in_block = static_cast(enc_slice[0]); } return Status::OK(); } @@ -391,14 +392,14 @@ Status BlockCacheHumanReadableTraceReader::ReadAccess( record->level = static_cast(ParseUint64(record_strs[6])); record->sst_fd_number = ParseUint64(record_strs[7]); record->caller = static_cast(ParseUint64(record_strs[8])); - record->no_insert = static_cast(ParseUint64(record_strs[9])); + record->no_insert = static_cast(ParseUint64(record_strs[9])); record->get_id = ParseUint64(record_strs[10]); uint64_t get_key_id = ParseUint64(record_strs[11]); record->referenced_data_size = ParseUint64(record_strs[12]); - record->is_cache_hit = static_cast(ParseUint64(record_strs[13])); + record->is_cache_hit = static_cast(ParseUint64(record_strs[13])); record->referenced_key_exist_in_block = - static_cast(ParseUint64(record_strs[14])); + static_cast(ParseUint64(record_strs[14])); record->num_keys_in_block = ParseUint64(record_strs[15]); uint64_t table_id = ParseUint64(record_strs[16]); if (table_id > 0) { @@ -408,7 +409,7 @@ Status BlockCacheHumanReadableTraceReader::ReadAccess( } uint64_t get_sequence_number = ParseUint64(record_strs[17]); if (get_sequence_number > 0) { - record->get_from_user_specified_snapshot = Boolean::kTrue; + record->get_from_user_specified_snapshot = true; // Decrement since valid seq number in the trace file equals traced seq // number + 1. get_sequence_number -= 1; @@ -445,16 +446,15 @@ BlockCacheTracer::BlockCacheTracer() { writer_.store(nullptr); } BlockCacheTracer::~BlockCacheTracer() { EndTrace(); } Status BlockCacheTracer::StartTrace( - SystemClock* clock, const TraceOptions& trace_options, - std::unique_ptr&& trace_writer) { + const BlockCacheTraceOptions& trace_options, + std::unique_ptr&& trace_writer) { InstrumentedMutexLock lock_guard(&trace_writer_mutex_); if (writer_.load()) { return Status::Busy(); } get_id_counter_.store(1); trace_options_ = trace_options; - writer_.store( - new BlockCacheTraceWriter(clock, trace_options, std::move(trace_writer))); + writer_.store(trace_writer.release()); return writer_.load()->WriteHeader(); } @@ -494,4 +494,11 @@ uint64_t BlockCacheTracer::NextGetId() { return prev_value; } +std::unique_ptr NewBlockCacheTraceWriter( + SystemClock* clock, const BlockCacheTraceWriterOptions& trace_options, + std::unique_ptr&& trace_writer) { + return std::unique_ptr(new BlockCacheTraceWriterImpl( + clock, trace_options, std::move(trace_writer))); +} + } // namespace ROCKSDB_NAMESPACE diff --git a/trace_replay/block_cache_tracer.h b/trace_replay/block_cache_tracer.h index feea5ad51..4a749608f 100644 --- a/trace_replay/block_cache_tracer.h +++ b/trace_replay/block_cache_tracer.h @@ -9,9 +9,10 @@ #include #include "monitoring/instrumented_mutex.h" +#include "rocksdb/block_cache_trace_writer.h" #include "rocksdb/options.h" +#include "rocksdb/table_reader_caller.h" #include "rocksdb/trace_reader_writer.h" -#include "table/table_reader_caller.h" #include "trace_replay/trace_replay.h" namespace ROCKSDB_NAMESPACE { @@ -102,65 +103,6 @@ struct BlockCacheLookupContext { } }; -enum Boolean : char { kTrue = 1, kFalse = 0 }; - -struct BlockCacheTraceRecord { - // Required fields for all accesses. - uint64_t access_timestamp = 0; - std::string block_key; - TraceType block_type = TraceType::kTraceMax; - uint64_t block_size = 0; - uint64_t cf_id = 0; - std::string cf_name; - uint32_t level = 0; - uint64_t sst_fd_number = 0; - TableReaderCaller caller = TableReaderCaller::kMaxBlockCacheLookupCaller; - Boolean is_cache_hit = Boolean::kFalse; - Boolean no_insert = Boolean::kFalse; - // Required field for Get and MultiGet - uint64_t get_id = BlockCacheTraceHelper::kReservedGetId; - Boolean get_from_user_specified_snapshot = Boolean::kFalse; - std::string referenced_key; - // Required fields for data block and user Get/Multi-Get only. - uint64_t referenced_data_size = 0; - uint64_t num_keys_in_block = 0; - Boolean referenced_key_exist_in_block = Boolean::kFalse; - - BlockCacheTraceRecord() {} - - BlockCacheTraceRecord( - uint64_t _access_timestamp, std::string _block_key, TraceType _block_type, - uint64_t _block_size, uint64_t _cf_id, std::string _cf_name, - uint32_t _level, uint64_t _sst_fd_number, TableReaderCaller _caller, - bool _is_cache_hit, bool _no_insert, - uint64_t _get_id = BlockCacheTraceHelper::kReservedGetId, - bool _get_from_user_specified_snapshot = false, - std::string _referenced_key = "", uint64_t _referenced_data_size = 0, - uint64_t _num_keys_in_block = 0, - bool _referenced_key_exist_in_block = false) - : access_timestamp(_access_timestamp), - block_key(_block_key), - block_type(_block_type), - block_size(_block_size), - cf_id(_cf_id), - cf_name(_cf_name), - level(_level), - sst_fd_number(_sst_fd_number), - caller(_caller), - is_cache_hit(_is_cache_hit ? Boolean::kTrue : Boolean::kFalse), - no_insert(_no_insert ? Boolean::kTrue : Boolean::kFalse), - get_id(_get_id), - get_from_user_specified_snapshot(_get_from_user_specified_snapshot - ? Boolean::kTrue - : Boolean::kFalse), - referenced_key(_referenced_key), - referenced_data_size(_referenced_data_size), - num_keys_in_block(_num_keys_in_block), - referenced_key_exist_in_block( - _referenced_key_exist_in_block ? Boolean::kTrue : Boolean::kFalse) { - } -}; - struct BlockCacheTraceHeader { uint64_t start_time; uint32_t rocksdb_major_version; @@ -171,16 +113,18 @@ struct BlockCacheTraceHeader { // user-provided TraceWriter. Every RocksDB operation is written as a single // trace. Each trace will have a timestamp and type, followed by the trace // payload. -class BlockCacheTraceWriter { +class BlockCacheTraceWriterImpl : public BlockCacheTraceWriter { public: - BlockCacheTraceWriter(SystemClock* clock, const TraceOptions& trace_options, - std::unique_ptr&& trace_writer); - ~BlockCacheTraceWriter() = default; + BlockCacheTraceWriterImpl(SystemClock* clock, + const BlockCacheTraceWriterOptions& trace_options, + std::unique_ptr&& trace_writer); + ~BlockCacheTraceWriterImpl() = default; // No copy and move. - BlockCacheTraceWriter(const BlockCacheTraceWriter&) = delete; - BlockCacheTraceWriter& operator=(const BlockCacheTraceWriter&) = delete; - BlockCacheTraceWriter(BlockCacheTraceWriter&&) = delete; - BlockCacheTraceWriter& operator=(BlockCacheTraceWriter&&) = delete; + BlockCacheTraceWriterImpl(const BlockCacheTraceWriterImpl&) = delete; + BlockCacheTraceWriterImpl& operator=(const BlockCacheTraceWriterImpl&) = + delete; + BlockCacheTraceWriterImpl(BlockCacheTraceWriterImpl&&) = delete; + BlockCacheTraceWriterImpl& operator=(BlockCacheTraceWriterImpl&&) = delete; // Pass Slice references to avoid copy. Status WriteBlockAccess(const BlockCacheTraceRecord& record, @@ -193,7 +137,7 @@ class BlockCacheTraceWriter { private: SystemClock* clock_; - TraceOptions trace_options_; + BlockCacheTraceWriterOptions trace_options_; std::unique_ptr trace_writer_; }; @@ -267,8 +211,8 @@ class BlockCacheTracer { BlockCacheTracer& operator=(BlockCacheTracer&&) = delete; // Start writing block cache accesses to the trace_writer. - Status StartTrace(SystemClock* clock, const TraceOptions& trace_options, - std::unique_ptr&& trace_writer); + Status StartTrace(const BlockCacheTraceOptions& trace_options, + std::unique_ptr&& trace_writer); // Stop writing block cache accesses to the trace_writer. void EndTrace(); @@ -285,7 +229,7 @@ class BlockCacheTracer { uint64_t NextGetId(); private: - TraceOptions trace_options_; + BlockCacheTraceOptions trace_options_; // A mutex protects the writer_. InstrumentedMutex trace_writer_mutex_; std::atomic writer_; diff --git a/trace_replay/block_cache_tracer_test.cc b/trace_replay/block_cache_tracer_test.cc index 9386ed718..f9d0773bf 100644 --- a/trace_replay/block_cache_tracer_test.cc +++ b/trace_replay/block_cache_tracer_test.cc @@ -74,17 +74,17 @@ class BlockCacheTracerTest : public testing::Test { record.caller = GetCaller(key_id); record.level = kLevel; record.sst_fd_number = kSSTFDNumber + key_id; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; // Provide get_id for all callers. The writer should only write get_id // when the caller is either GET or MGET. record.get_id = key_id + 1; - record.get_from_user_specified_snapshot = Boolean::kTrue; + record.get_from_user_specified_snapshot = true; // Provide these fields for all block types. // The writer should only write these fields for data blocks and the // caller is either GET or MGET. record.referenced_key = (kRefKeyPrefix + std::to_string(key_id)); - record.referenced_key_exist_in_block = Boolean::kTrue; + record.referenced_key_exist_in_block = true; record.num_keys_in_block = kNumKeysInBlock; record.referenced_data_size = kReferencedDataSize + key_id; ASSERT_OK(writer->WriteBlockAccess( @@ -104,10 +104,10 @@ class BlockCacheTracerTest : public testing::Test { record.caller = GetCaller(key_id); record.level = kLevel; record.sst_fd_number = kSSTFDNumber + key_id; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; record.referenced_key = kRefKeyPrefix + std::to_string(key_id); - record.referenced_key_exist_in_block = Boolean::kTrue; + record.referenced_key_exist_in_block = true; record.num_keys_in_block = kNumKeysInBlock; return record; } @@ -127,28 +127,28 @@ class BlockCacheTracerTest : public testing::Test { ASSERT_EQ(GetCaller(key_id), record.caller); ASSERT_EQ(kLevel, record.level); ASSERT_EQ(kSSTFDNumber + key_id, record.sst_fd_number); - ASSERT_EQ(Boolean::kFalse, record.is_cache_hit); - ASSERT_EQ(Boolean::kFalse, record.no_insert); + ASSERT_FALSE(record.is_cache_hit); + ASSERT_FALSE(record.no_insert); if (record.caller == TableReaderCaller::kUserGet || record.caller == TableReaderCaller::kUserMultiGet) { ASSERT_EQ(key_id + 1, record.get_id); - ASSERT_EQ(Boolean::kTrue, record.get_from_user_specified_snapshot); + ASSERT_TRUE(record.get_from_user_specified_snapshot); ASSERT_EQ(kRefKeyPrefix + std::to_string(key_id), record.referenced_key); } else { ASSERT_EQ(BlockCacheTraceHelper::kReservedGetId, record.get_id); - ASSERT_EQ(Boolean::kFalse, record.get_from_user_specified_snapshot); + ASSERT_FALSE(record.get_from_user_specified_snapshot); ASSERT_EQ("", record.referenced_key); } if (block_type == TraceType::kBlockTraceDataBlock && (record.caller == TableReaderCaller::kUserGet || record.caller == TableReaderCaller::kUserMultiGet)) { - ASSERT_EQ(Boolean::kTrue, record.referenced_key_exist_in_block); + ASSERT_TRUE(record.referenced_key_exist_in_block); ASSERT_EQ(kNumKeysInBlock, record.num_keys_in_block); ASSERT_EQ(kReferencedDataSize + key_id, record.referenced_data_size); continue; } - ASSERT_EQ(Boolean::kFalse, record.referenced_key_exist_in_block); + ASSERT_FALSE(record.referenced_key_exist_in_block); ASSERT_EQ(0, record.num_keys_in_block); ASSERT_EQ(0, record.referenced_data_size); } @@ -188,12 +188,18 @@ TEST_F(BlockCacheTracerTest, AtomicWriteBeforeStartTrace) { TEST_F(BlockCacheTracerTest, AtomicWrite) { BlockCacheTraceRecord record = GenerateAccessRecord(); { - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; + BlockCacheTraceOptions trace_opt; std::unique_ptr trace_writer; ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); BlockCacheTracer writer; - ASSERT_OK(writer.StartTrace(clock_, trace_opt, std::move(trace_writer))); + ASSERT_OK( + writer.StartTrace(trace_opt, std::move(block_cache_trace_writer))); ASSERT_OK(writer.WriteBlockAccess(record, record.block_key, record.cf_name, record.referenced_key)); ASSERT_OK(env_->FileExists(trace_file_path_)); @@ -214,25 +220,36 @@ TEST_F(BlockCacheTracerTest, AtomicWrite) { } TEST_F(BlockCacheTracerTest, ConsecutiveStartTrace) { - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; + BlockCacheTraceOptions trace_opt; std::unique_ptr trace_writer; ASSERT_OK( NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); BlockCacheTracer writer; - ASSERT_OK(writer.StartTrace(clock_, trace_opt, std::move(trace_writer))); - ASSERT_NOK(writer.StartTrace(clock_, trace_opt, std::move(trace_writer))); + ASSERT_OK(writer.StartTrace(trace_opt, std::move(block_cache_trace_writer))); + ASSERT_NOK(writer.StartTrace(trace_opt, std::move(block_cache_trace_writer))); ASSERT_OK(env_->FileExists(trace_file_path_)); } TEST_F(BlockCacheTracerTest, AtomicNoWriteAfterEndTrace) { BlockCacheTraceRecord record = GenerateAccessRecord(); { - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; + BlockCacheTraceOptions trace_opt; std::unique_ptr trace_writer; ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); BlockCacheTracer writer; - ASSERT_OK(writer.StartTrace(clock_, trace_opt, std::move(trace_writer))); + ASSERT_OK( + writer.StartTrace(trace_opt, std::move(block_cache_trace_writer))); ASSERT_OK(writer.WriteBlockAccess(record, record.block_key, record.cf_name, record.referenced_key)); writer.EndTrace(); @@ -260,14 +277,20 @@ TEST_F(BlockCacheTracerTest, AtomicNoWriteAfterEndTrace) { TEST_F(BlockCacheTracerTest, NextGetId) { BlockCacheTracer writer; { - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; + BlockCacheTraceOptions trace_opt; std::unique_ptr trace_writer; ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); // next get id should always return 0 before we call StartTrace. ASSERT_EQ(0, writer.NextGetId()); ASSERT_EQ(0, writer.NextGetId()); - ASSERT_OK(writer.StartTrace(clock_, trace_opt, std::move(trace_writer))); + ASSERT_OK( + writer.StartTrace(trace_opt, std::move(block_cache_trace_writer))); ASSERT_EQ(1, writer.NextGetId()); ASSERT_EQ(2, writer.NextGetId()); writer.EndTrace(); @@ -277,11 +300,17 @@ TEST_F(BlockCacheTracerTest, NextGetId) { // Start trace again and next get id should return 1. { - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; + BlockCacheTraceOptions trace_opt; std::unique_ptr trace_writer; ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); - ASSERT_OK(writer.StartTrace(clock_, trace_opt, std::move(trace_writer))); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); + ASSERT_OK( + writer.StartTrace(trace_opt, std::move(block_cache_trace_writer))); ASSERT_EQ(1, writer.NextGetId()); } } @@ -289,19 +318,26 @@ TEST_F(BlockCacheTracerTest, NextGetId) { TEST_F(BlockCacheTracerTest, MixedBlocks) { { // Generate a trace file containing a mix of blocks. - TraceOptions trace_opt; + BlockCacheTraceWriterOptions trace_writer_opt; std::unique_ptr trace_writer; ASSERT_OK(NewFileTraceWriter(env_, env_options_, trace_file_path_, &trace_writer)); - BlockCacheTraceWriter writer(clock_, trace_opt, std::move(trace_writer)); - ASSERT_OK(writer.WriteHeader()); + std::unique_ptr block_cache_trace_writer = + NewBlockCacheTraceWriter(env_->GetSystemClock().get(), trace_writer_opt, + std::move(trace_writer)); + ASSERT_NE(block_cache_trace_writer, nullptr); + ASSERT_OK(block_cache_trace_writer->WriteHeader()); // Write blocks of different types. - WriteBlockAccess(&writer, 0, TraceType::kBlockTraceUncompressionDictBlock, - 10); - WriteBlockAccess(&writer, 10, TraceType::kBlockTraceDataBlock, 10); - WriteBlockAccess(&writer, 20, TraceType::kBlockTraceFilterBlock, 10); - WriteBlockAccess(&writer, 30, TraceType::kBlockTraceIndexBlock, 10); - WriteBlockAccess(&writer, 40, TraceType::kBlockTraceRangeDeletionBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 0, + TraceType::kBlockTraceUncompressionDictBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 10, + TraceType::kBlockTraceDataBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 20, + TraceType::kBlockTraceFilterBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 30, + TraceType::kBlockTraceIndexBlock, 10); + WriteBlockAccess(block_cache_trace_writer.get(), 40, + TraceType::kBlockTraceRangeDeletionBlock, 10); ASSERT_OK(env_->FileExists(trace_file_path_)); } @@ -332,7 +368,7 @@ TEST_F(BlockCacheTracerTest, HumanReadableTrace) { record.get_id = 1; record.referenced_key = ""; record.caller = TableReaderCaller::kUserGet; - record.get_from_user_specified_snapshot = Boolean::kTrue; + record.get_from_user_specified_snapshot = true; record.referenced_data_size = kReferencedDataSize; PutFixed32(&record.referenced_key, 111); PutLengthPrefixedSlice(&record.referenced_key, "get_key"); @@ -359,11 +395,11 @@ TEST_F(BlockCacheTracerTest, HumanReadableTrace) { ASSERT_EQ(TableReaderCaller::kUserGet, read_record.caller); ASSERT_EQ(kLevel, read_record.level); ASSERT_EQ(kSSTFDNumber, read_record.sst_fd_number); - ASSERT_EQ(Boolean::kFalse, read_record.is_cache_hit); - ASSERT_EQ(Boolean::kFalse, read_record.no_insert); + ASSERT_FALSE(read_record.is_cache_hit); + ASSERT_FALSE(read_record.no_insert); ASSERT_EQ(1, read_record.get_id); - ASSERT_EQ(Boolean::kTrue, read_record.get_from_user_specified_snapshot); - ASSERT_EQ(Boolean::kTrue, read_record.referenced_key_exist_in_block); + ASSERT_TRUE(read_record.get_from_user_specified_snapshot); + ASSERT_TRUE(read_record.referenced_key_exist_in_block); ASSERT_EQ(kNumKeysInBlock, read_record.num_keys_in_block); ASSERT_EQ(kReferencedDataSize, read_record.referenced_data_size); ASSERT_EQ(record.block_key.size(), read_record.block_key.size()); diff --git a/utilities/simulator_cache/cache_simulator.cc b/utilities/simulator_cache/cache_simulator.cc index b214d4ac0..dc419e51a 100644 --- a/utilities/simulator_cache/cache_simulator.cc +++ b/utilities/simulator_cache/cache_simulator.cc @@ -41,7 +41,7 @@ void CacheSimulator::Access(const BlockCacheTraceRecord& access) { const bool is_user_access = BlockCacheTraceHelper::IsUserAccess(access.caller); bool is_cache_miss = true; - if (ghost_cache_ && access.no_insert == Boolean::kFalse) { + if (ghost_cache_ && !access.no_insert) { admit = ghost_cache_->Admit(access.block_key); } auto handle = sim_cache_->Lookup(access.block_key); @@ -49,7 +49,7 @@ void CacheSimulator::Access(const BlockCacheTraceRecord& access) { sim_cache_->Release(handle); is_cache_miss = false; } else { - if (access.no_insert == Boolean::kFalse && admit && access.block_size > 0) { + if (!access.no_insert && admit && access.block_size > 0) { // Ignore errors on insert auto s = sim_cache_->Insert(access.block_key, /*value=*/nullptr, access.block_size, diff --git a/utilities/simulator_cache/cache_simulator_test.cc b/utilities/simulator_cache/cache_simulator_test.cc index 01441de46..2bc057c92 100644 --- a/utilities/simulator_cache/cache_simulator_test.cc +++ b/utilities/simulator_cache/cache_simulator_test.cc @@ -43,11 +43,11 @@ class CacheSimulatorTest : public testing::Test { record.level = 6; record.sst_fd_number = 0; record.get_id = getid; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kFalse; + record.is_cache_hit = false; + record.no_insert = false; record.referenced_key = kRefKeyPrefix + std::to_string(kGetId) + kRefKeySequenceNumber; - record.referenced_key_exist_in_block = Boolean::kTrue; + record.referenced_key_exist_in_block = true; record.referenced_data_size = 100; record.num_keys_in_block = 300; return record; @@ -64,8 +64,8 @@ class CacheSimulatorTest : public testing::Test { record.caller = TableReaderCaller::kCompaction; record.level = 6; record.sst_fd_number = kCompactionBlockId; - record.is_cache_hit = Boolean::kFalse; - record.no_insert = Boolean::kTrue; + record.is_cache_hit = false; + record.no_insert = true; return record; } @@ -200,17 +200,17 @@ TEST_F(CacheSimulatorTest, GhostPrioritizedCacheSimulator) { TEST_F(CacheSimulatorTest, HybridRowBlockCacheSimulator) { uint64_t block_id = 100; BlockCacheTraceRecord first_get = GenerateGetRecord(kGetId); - first_get.get_from_user_specified_snapshot = Boolean::kTrue; + first_get.get_from_user_specified_snapshot = true; BlockCacheTraceRecord second_get = GenerateGetRecord(kGetId + 1); second_get.referenced_data_size = 0; - second_get.referenced_key_exist_in_block = Boolean::kFalse; - second_get.get_from_user_specified_snapshot = Boolean::kTrue; + second_get.referenced_key_exist_in_block = false; + second_get.get_from_user_specified_snapshot = true; BlockCacheTraceRecord third_get = GenerateGetRecord(kGetId + 2); third_get.referenced_data_size = 0; - third_get.referenced_key_exist_in_block = Boolean::kFalse; + third_get.referenced_key_exist_in_block = false; third_get.referenced_key = kRefKeyPrefix + "third_get"; // We didn't find the referenced key in the third get. - third_get.referenced_key_exist_in_block = Boolean::kFalse; + third_get.referenced_key_exist_in_block = false; third_get.referenced_data_size = 0; std::shared_ptr sim_cache = NewLRUCache(/*capacity=*/kCacheSize, /*num_shard_bits=*/1, @@ -308,12 +308,12 @@ TEST_F(CacheSimulatorTest, HybridRowBlockCacheSimulatorGetTest) { get.access_timestamp = 0; get.block_key = "1"; get.get_id = 1; - get.get_from_user_specified_snapshot = Boolean::kFalse; + get.get_from_user_specified_snapshot = false; get.referenced_key = kRefKeyPrefix + std::to_string(1) + kRefKeySequenceNumber; - get.no_insert = Boolean::kFalse; + get.no_insert = false; get.sst_fd_number = 0; - get.get_from_user_specified_snapshot = Boolean::kFalse; + get.get_from_user_specified_snapshot = false; LRUCacheOptions co; co.capacity = 16;