base,sstable: include {loaded,cached} block counts... #1729
Conversation
|
This change is |
...in iterator stats. Similar to the byte counts exported in cockroachdb#1538, these numbers can come in handy when reasoning about block cache hit rates for slow scans. Fix up some stat pretty printing units while here.
irfansharif
force-pushed
the
220527.export-block-counts
branch
from
10f7fea
to
41ca3d2
Compare
Adds plumbing for surfacing the # of blocks read in total + from the pebble block cache, things we're now tracking down in pebble: github.com/cockroachdb/pebble/pull/1729. These numbers can come in handy when reasoning about block cache hit rates for slow scans. Release note: None
There was a problem hiding this comment.
Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @irfansharif and @jbowens)
internal/base/iterator.go line 243 at r1 (raw file):
// the compressed bytes. Currently, only the second-level index and data // blocks containing points are included. Blocks, BlockBytes uint64
I think its fine to try this out for experiments (I tend to copy experimental Pebble code into vendor and build a binary). But unless we have some data that shows that using BlockBytesInCache/BlockBytes is flawed as a cache hit rate metric, and we need BlocksInCache/Blocks, I would hold off on adding these counts to the master branch.
It creates overlap and potential confusion on what to look at.
I realize we have existing rocksdb.block.cache.hits and rocksdb.block.cache.misses metrics in CockroachDB, that are in terms of count. I think for the experiments with the latency spike we shouldn't try to compare with those and instead compare BlockBytesInCache/BlockBytes during the spike with that when there is no latency spike, for the SQL queries.
...in iterator stats. Similar to the byte counts exported in #1538,
these numbers can come in handy when reasoning about block cache hit
rates for slow scans. Fix up some stat pretty printing units while here.