topsql_v2: add detailed TiKV I/O dimensions - #96
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
The updated Top-N cutoff logic has a tie-handling bug and also risks u64 overflow when summing network bytes, which can lead to incorrect filtering/aggregation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR extends the TiKV TopSQL upstream parser to decode and emit additional resource-dimension metrics (network I/O bytes, logical read/write bytes, RocksDB block read count) while preserving existing behaviors like per-second Top-N filtering, others aggregation, and downsampling.
Changes:
- Extend
GroupTagRecordItem(protobuf + Rust handling) to include 5 new TiKV resource usage fields. - Emit new TopSQL metrics for the added resource dimensions.
- Update Top-N filtering,
othersmerging behavior, and downsampling logic/tests to account for the new dimensions.
File summaries
| File | Description |
|---|---|
src/sources/topsql/upstream/tikv/parser.rs |
Adds field merges, resource-aware Top-N selection, emits new metrics, and extends downsampling/tests. |
src/sources/topsql/upstream/tikv/mock_upstream.rs |
Extends mock TiKV metering records with the new resource fields. |
src/sources/topsql/upstream/consts.rs |
Adds metric name constants for the new TiKV resource dimensions. |
proto/tikv.proto |
Extends the protobuf message with new fields and field numbers (5–9). |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| let kth_cpu = kth_largest(v.iter().map(|psd| psd.cpu_time_ms), top_n); | ||
| let kth_network = kth_largest( | ||
| v.iter() | ||
| .map(|psd| psd.network_in_bytes + psd.network_out_bytes), | ||
| top_n, | ||
| ); | ||
| let kth_logical_read = kth_largest(v.iter().map(|psd| psd.logical_read_bytes), top_n); | ||
| let kth_logical_write = kth_largest(v.iter().map(|psd| psd.logical_write_bytes), top_n); | ||
| let kth_block_read = | ||
| kth_largest(v.iter().map(|psd| psd.rocksdb_block_read_count), top_n); | ||
|
|
||
| let mut kept = Vec::with_capacity(v.len()); | ||
| for psd in std::mem::take(v) { | ||
| if psd.cpu_time_ms > kth_cpu | ||
| || psd.network_in_bytes + psd.network_out_bytes > kth_network | ||
| || psd.logical_read_bytes > kth_logical_read | ||
| || psd.logical_write_bytes > kth_logical_write | ||
| || psd.rocksdb_block_read_count > kth_block_read | ||
| { |
Signed-off-by: jiong-nba <jiongnba@gmail.com>
1af8155 to
b701ade
Compare
a716bd3 to
b701ade
Compare
TiKV TopSQL Detailed I/O Dimensions
Overview
This PR updates the
0.49TopSQL v2 pipeline to consume and persist the detailed TiKV I/O dimensions introduced by tikv/tikv#19837 and pingcap/kvproto#1498.It covers the dedicated cloud path implemented on the
0.49branch:TiKV -> topsql_v2 -> topsql_data_deltalake -> Delta LakeChanges
Protocol
rocksdb_block_read_count = 9toGroupTagRecordItem.0when the field is absent.TopSQL v2 source
rocksdb_block_read_countfor bothtikv_topsqlandtikv_topregionevents.othersrecords and during downsampling.Delta Lake sink
topsql_rocksdb_block_read_countto the TopSQL data schema.Metric semantics
rocksdb_block_read_countis the number of foreground RocksDB block reads attributed to a request. It is useful for relative TopSQL ranking, but it is not device-level Read IOPS and should not be presented as an exact physical I/O count.Validation
Related changes: