Skip to content

Add TiKV resource dimensions for topsql - #96

Open
jiong-nba wants to merge 1 commit into
tidbcloud:masterfrom
jiong-nba:codex/add-tikv-topsql-resource-dimensions
Open

Add TiKV resource dimensions for topsql#96
jiong-nba wants to merge 1 commit into
tidbcloud:masterfrom
jiong-nba:codex/add-tikv-topsql-resource-dimensions

Conversation

@jiong-nba

Copy link
Copy Markdown

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, others aggregation, downsampling, and emitted TopSQL metrics consistent for the following fields:

  • Network input/output bytes
  • Logical read/write bytes
  • RocksDB block read count

Key Features

1. Extended TiKV Resource Usage Protocol

Add the following fields to GroupTagRecordItem using the field numbers defined by kvproto:

  • network_in_bytes = 5
  • network_out_bytes = 6
  • logical_read_bytes = 7
  • logical_write_bytes = 8
  • rocksdb_block_read_count = 9

2. TopSQL Metric Output

Export the new TiKV resource dimensions as TopSQL metrics:

  • topsql_network_in_bytes
  • topsql_network_out_bytes
  • topsql_logical_read_bytes
  • topsql_logical_write_bytes
  • topsql_rocksdb_block_read_count

3. Resource-Aware Top N Filtering

  • Select candidates independently by CPU, combined network traffic, logical reads, logical writes, and RocksDB block reads.
  • Keep the union of candidates selected by each resource dimension, matching TiKV's resource metering semantics.
  • Merge all fields from evicted records into the per-second others record.
  • Merge multiple existing others records at the same timestamp instead of overwriting them.
  • Preserve all resource dimensions during downsampling.

Tests

cargo test --lib sources::topsql::upstream::tikv::parser::tests -- --nocapture
cargo test --lib --features nextgen sources::topsql::upstream::tikv::parser::tests -- --nocapture
cargo check --workspace --all-targets
cargo check --workspace --all-targets --features nextgen
cargo fmt --all -- --check
git diff --check

Related changes:

Signed-off-by: Jiong <jiongnba@gmail.com>
Copilot AI review requested due to automatic review settings August 3, 2026 08:33
@pingcap-cla-assistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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, others merging 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.

Comment on lines +149 to +167
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
{
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants