[RFC] Refactor block cache tracing APIs #10811

akankshamahajan15 · 2022-10-12T23:00:01Z

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.

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

Reviewers:

Subscribers:

Tasks:

Tags:

akankshamahajan15 · 2022-10-12T23:02:51Z

Referring PR #9326

facebook-github-bot · 2022-10-17T16:39:33Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

include/rocksdb/db.h

include/rocksdb/block_cache_trace_writer.h

facebook-github-bot · 2022-10-20T17:47:40Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-10-20T18:42:49Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-10-20T18:53:36Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

anand1976

LGTM. Update HISTORY.md.

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. This same philosophy can be applied to KV and IO tracing as well. Test Plan: existing unit tests Reviewers: Subscribers: Tasks: Tags:

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

facebook-github-bot · 2022-10-21T02:50:10Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-10-21T02:50:43Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

ajkr · 2022-10-21T07:20:18Z

include/rocksdb/utilities/stackable_db.h

+
+  Status StartBlockCacheTrace(
+      const BlockCacheTraceOptions& options,
+      std::unique_ptr<BlockCacheTraceWriter>&& trace_writer) override {


What if StartBlockCacheTrace() accepts a TraceRecord::Handler for users who want to do custom record tracing?

I am not sure about that approach as I am not familiar with the query tracing. Let me take a look at it. cc @anand1976 Do you have any thoughts on it.

Yeah it would be pretty different. I can try that approach for KV tracing and see how it looks. Feel free to land this now if you need it in 7.8 release as I think it'll take a while to investigate.

I checked the code and it seems its more involved with Handler as the process for query tracing is quite different from block cache tracing.

What if StartBlockCacheTrace() accepts a TraceRecord::Handler for users who want to do custom record tracing?

It's probably more general than needed for this PR's stated purpose. The downside to generalizing the write side so much is the read side can't make any assumptions, e.g., that a user can provide a TraceReader to read a trace. This PR doesn't handle the read side either but I think the design should allow it. However, you would need to introduce yet another independent interface (BlockCacheTraceReader).

What if instead we made a TraceCodec interface that defines the string<->TraceRecord conversion? It could be provided together with TraceWriter to all the write functions, and provided together with TraceReader to all the read functions. That way there is just one new interface for the combinations of (block cache, KV, I/O) x (read, write).

What's the string<->TraceRecord conversion? How it would be used?

For example, a TraceCodec would include the following functions:

class TraceCodec { ... virtual std::string Encode(const BlockCacheTraceRecord&) = 0; virtual BlockCacheTraceRecord Decode(const std::string&) = 0; ... };

Then BlockCacheTraceWriter can be internal again, but needs to be configured with a TraceCodec that comes from StartBlockCacheTrace(). Say we store it in an instance variable, trace_codec_. Then BlockCacheTraceWriter can use it like this:

Status BlockCacheTraceWriter::WriteBlockAccess( const BlockCacheTraceRecord& record) { return trace_writer_->Write(trace_codec_->Encode(record)); }

Ah ok. That's something that can be done. I will give it a shot and see how it goes.

Just to confirm, is your client use case OK with using TraceWriter for writing the physical trace records? If not, something like the BlockCacheTraceWriter seems necessary to customize writing logical trace records.

edit: giving users the ability to customize writing logical trace records (what you did here) could be a good approach in any case. I am mostly interested in unifying the class hierarchies so will think more how that would look for this approach.

ajkr · 2022-10-21T07:20:44Z

include/rocksdb/block_cache_trace_writer.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 {


Can it be in the TraceRecord class hierarchy?

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: facebook#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

facebook-github-bot added the CLA Signed label Oct 12, 2022

akankshamahajan15 requested a review from anand1976 October 17, 2022 16:38

anand1976 reviewed Oct 19, 2022

View reviewed changes

include/rocksdb/db.h Show resolved Hide resolved

include/rocksdb/block_cache_trace_writer.h Outdated Show resolved Hide resolved

akankshamahajan15 force-pushed the block_cache_ branch from 69f2310 to 8b8e3ca Compare October 20, 2022 17:47

akankshamahajan15 requested a review from anand1976 October 20, 2022 20:20

anand1976 approved these changes Oct 20, 2022

View reviewed changes

akankshamahajan15 added 4 commits October 20, 2022 19:46

Addressed comments

da9dd10

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

Remove filter from BlockCacheTraceOptions

f88a115

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

Add History.md

f6e275e

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

akankshamahajan15 force-pushed the block_cache_ branch from da65306 to f6e275e Compare October 21, 2022 02:50

ajkr reviewed Oct 21, 2022

View reviewed changes

facebook-github-bot closed this in 0e7b27b Oct 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Refactor block cache tracing APIs #10811

[RFC] Refactor block cache tracing APIs #10811

akankshamahajan15 commented Oct 12, 2022 •

edited

Loading

akankshamahajan15 commented Oct 12, 2022

facebook-github-bot commented Oct 17, 2022

facebook-github-bot commented Oct 20, 2022

facebook-github-bot commented Oct 20, 2022

facebook-github-bot commented Oct 20, 2022

anand1976 left a comment

facebook-github-bot commented Oct 21, 2022

facebook-github-bot commented Oct 21, 2022

ajkr Oct 21, 2022

akankshamahajan15 Oct 21, 2022 •

edited

Loading

ajkr Oct 21, 2022

akankshamahajan15 Oct 21, 2022

ajkr Oct 21, 2022

akankshamahajan15 Oct 21, 2022

ajkr Oct 21, 2022

akankshamahajan15 Oct 21, 2022

ajkr Oct 22, 2022 •

edited

Loading

ajkr Oct 21, 2022

[RFC] Refactor block cache tracing APIs #10811

[RFC] Refactor block cache tracing APIs #10811

Conversation

akankshamahajan15 commented Oct 12, 2022 • edited Loading

akankshamahajan15 commented Oct 12, 2022

facebook-github-bot commented Oct 17, 2022

facebook-github-bot commented Oct 20, 2022

facebook-github-bot commented Oct 20, 2022

facebook-github-bot commented Oct 20, 2022

anand1976 left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 21, 2022

facebook-github-bot commented Oct 21, 2022

Choose a reason for hiding this comment

akankshamahajan15 Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajkr Oct 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akankshamahajan15 commented Oct 12, 2022 •

edited

Loading

akankshamahajan15 Oct 21, 2022 •

edited

Loading

ajkr Oct 22, 2022 •

edited

Loading