Add TiKV resource dimensions for topsql - #96
Conversation
Signed-off-by: Jiong <jiongnba@gmail.com>
|
|
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 | ||
| { |
TiKV TopSQL Resource Dimensions
Overview
This PR extends the existing TiKV TopSQL source to consume and export the resource dimensions introduced by tikv/tikv#19837.
It keeps protobuf decoding, per-second Top N filtering,
othersaggregation, downsampling, and emitted TopSQL metrics consistent for the following fields:Key Features
1. Extended TiKV Resource Usage Protocol
Add the following fields to
GroupTagRecordItemusing the field numbers defined by kvproto:network_in_bytes = 5network_out_bytes = 6logical_read_bytes = 7logical_write_bytes = 8rocksdb_block_read_count = 92. TopSQL Metric Output
Export the new TiKV resource dimensions as TopSQL metrics:
topsql_network_in_bytestopsql_network_out_bytestopsql_logical_read_bytestopsql_logical_write_bytestopsql_rocksdb_block_read_count3. Resource-Aware Top N Filtering
othersrecord.othersrecords at the same timestamp instead of overwriting them.Tests
Related changes: