metrics: expose IA remote-read scan details#69606
Conversation
|
Skipping CI for Draft Pull Request. |
ti-chi-bot
Bot
added
do-not-merge/work-in-progress
📝 WalkthroughWalkthroughThis PR adds IA (remote) read segment metrics—count, size, and wait time—end-to-end: a new extraction helper in execdetails, Prometheus counters/histogram, slow query log fields and columns, and statement summary (v1/v2) aggregation, along with accompanying schema updates and tests. ChangesIA Remote Read Segment Metrics
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Conn as conn.go addQueryMetrics
participant ExecDetails as execdetails.GetIARemoteReadSegmentStats
participant Metrics as IARemoteReadSegmentCount/Size/WaitDuration
Conn->>ExecDetails: extract stats from ScanDetail
ExecDetails-->>Conn: IARemoteReadSegmentStats
Conn->>Metrics: observe count/size/wait duration
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
the
size/XL
Connor1996
force-pushed
the
codex/tidb-ia-remote-read-observability
branch
from
cbda601 to
53462ff
Compare
ti-chi-bot
Bot
added
size/XXL
Connor1996
force-pushed
the
codex/tidb-ia-remote-read-observability
branch
from
53462ff to
9506fba
Compare
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Connor1996
force-pushed
the
codex/tidb-ia-remote-read-observability
branch
from
9506fba to
d136bb6
Compare
ti-chi-bot
Bot
added
release-note
|
@Connor1996: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/util/execdetails/execdetails.go`:
- Around line 213-231: The new IA remote-read fields are not available in the
current client-go version, so the added accessor in GetIARemoteReadSegmentStats
will not compile yet. Hold this change until the github.com/tikv/client-go/v2
bump lands, or guard/remove the references to
util.ScanDetail.IaRemoteReadSegmentCount, IaRemoteReadSegmentBytes, and
IaRemoteReadSegmentDuration so this package and its tests only use fields that
exist in the current dependency.
In `@pkg/util/stmtsummary/statement_summary.go`:
- Around line 202-207: The field naming is inconsistent across the statement
summary stats, where sumIAReadSegmentCount/maxIAReadSegmentCount omit “Remote”
while the related fields and upstream source use IARemoteReadSegment/
IaRemoteReadSegmentCount. Rename the struct fields in statement_summary.go and
update all matching references in reader.go and tables.go/v2 so the exported SQL
column becomes AVG_IA_REMOTE_READ_SEGMENT_COUNT (and related names stay
aligned). Use the existing sibling symbols like sumIARemoteReadSegmentSize,
maxIARemoteReadSegmentWaitTime, and GetIARemoteReadSegmentStats as the naming
reference when updating the public INFORMATION_SCHEMA.STATEMENTS_SUMMARY
columns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9766269a-88a6-49f6-bd9b-cf863d6f288b
📒 Files selected for processing (21)
pkg/executor/slow_query.gopkg/executor/slow_query_test.gopkg/infoschema/metric_table_def.gopkg/infoschema/tables.gopkg/infoschema/test/clustertablestest/tables_test.gopkg/metrics/metrics.gopkg/metrics/server.gopkg/server/conn.gopkg/sessionctx/variable/slow_log.gopkg/sessionctx/variable/tests/session_test.gopkg/util/execdetails/execdetails.gopkg/util/execdetails/execdetails_test.gopkg/util/stmtsummary/BUILD.bazelpkg/util/stmtsummary/evicted.gopkg/util/stmtsummary/reader.gopkg/util/stmtsummary/statement_summary.gopkg/util/stmtsummary/statement_summary_test.gopkg/util/stmtsummary/v2/BUILD.bazelpkg/util/stmtsummary/v2/column.gopkg/util/stmtsummary/v2/column_test.gopkg/util/stmtsummary/v2/record.go
| // IARemoteReadSegmentStats contains IA remote-read scan statistics. | ||
| type IARemoteReadSegmentStats struct { | ||
| Count uint64 | ||
| Bytes uint64 | ||
| WaitTime time.Duration | ||
| } | ||
|
|
||
| // GetIARemoteReadSegmentStats reads IA remote-read scan statistics from client-go ScanDetail. | ||
| func GetIARemoteReadSegmentStats(scanDetail *util.ScanDetail) IARemoteReadSegmentStats { | ||
| if scanDetail == nil { | ||
| return IARemoteReadSegmentStats{} | ||
| } | ||
| return IARemoteReadSegmentStats{ | ||
| Count: scanDetail.IaRemoteReadSegmentCount, | ||
| Bytes: scanDetail.IaRemoteReadSegmentBytes, | ||
| WaitTime: scanDetail.IaRemoteReadSegmentDuration, | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== go.mod client-go dependency ==\n'
grep -nE 'tikv/client-go|client-go' go.mod || true
printf '\n== execdetails.go relevant lines ==\n'
sed -n '180,260p' pkg/util/execdetails/execdetails.go
printf '\n== ScanDetail field references in repo ==\n'
rg -n 'IaRemoteReadSegment|IARemoteReadSegment' -S .
printf '\n== client-go module cache probe (if available) ==\n'
go env GOPATH GOMODCACHE 2>/dev/null || trueRepository: pingcap/tidb
Length of output: 21160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
GOMODCACHE="$(go env GOMODCACHE)"
printf 'GOMODCACHE=%s\n' "$GOMODCACHE"
printf '\n== client-go module versions ==\n'
fd -a 'client-go' "$GOMODCACHE/github.com/tikv" 2>/dev/null || true
printf '\n== ScanDetail definition ==\n'
SCANDETAIL_FILE="$(rg -l -n 'type ScanDetail struct' "$GOMODCACHE/github.com/tikv/client-go" 2>/dev/null | head -n 1 || true)"
printf 'file=%s\n' "${SCANDETAIL_FILE:-<not found>}"
if [ -n "${SCANDETAIL_FILE:-}" ]; then
sed -n '1,260p' "$SCANDETAIL_FILE"
fi
printf '\n== IaRemoteReadSegment field references in client-go ==\n'
rg -n 'IaRemoteReadSegment(Count|Bytes|Duration)' "$GOMODCACHE/github.com/tikv/client-go" 2>/dev/null || trueRepository: pingcap/tidb
Length of output: 9764
🏁 Script executed:
#!/bin/bash
set -euo pipefail
GOMODCACHE="$(go env GOMODCACHE)"
CLIENT_GO_DIR="$GOMODCACHE/github.com/tikv/client-go/v2@v2.0.8-0.20260617030124-661db4f5f4e8"
printf '== ScanDetail struct location ==\n'
rg -n 'type ScanDetail struct' "$CLIENT_GO_DIR" -S
printf '\n== ScanDetail struct body ==\n'
SCAN_FILE="$(rg -l 'type ScanDetail struct' "$CLIENT_GO_DIR" | head -n 1)"
sed -n '/type ScanDetail struct/,/^}/p' "$SCAN_FILE"
printf '\n== IaRemoteReadSegment fields in ScanDetail body ==\n'
sed -n '/type ScanDetail struct/,/^}/p' "$SCAN_FILE" | rg -n 'IaRemoteReadSegment|IARemoteReadSegment' -n || trueRepository: pingcap/tidb
Length of output: 1852
Wait for the client-go bump before merging this change.
util.ScanDetail in the current github.com/tikv/client-go/v2 dependency still has no IaRemoteReadSegmentCount, IaRemoteReadSegmentBytes, or IaRemoteReadSegmentDuration fields, so this code and its tests will not compile until that dependency update lands.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 226-226: : # github.com/pingcap/tidb/pkg/util/execdetails [github.com/pingcap/tidb/pkg/util/execdetails.test]
pkg/util/execdetails/execdetails.go:226:24: scanDetail.IaRemoteReadSegmentCount undefined (type *"github.com/tikv/client-go/v2/util".ScanDetail has no field or method IaRemoteReadSegmentCount)
pkg/util/execdetails/execdetails.go:227:24: scanDetail.IaRemoteReadSegmentBytes undefined (type *"github.com/tikv/client-go/v2/util".ScanDetail has no field or method IaRemoteReadSegmentBytes)
pkg/util/execdetails/execdetails.go:228:24: scanDetail.IaRemoteReadSegmentDuration undefined (type *"github.com/tikv/client-go/v2/util".ScanDetail has no field or method IaRemoteReadSegmentDuration)
pkg/util/execdetails/execdetails_test.go:1194:3: unknown field IaRemoteReadSegmentCount in struct literal of type "github.com/tikv/client-go/v2/util".ScanDetail
pkg/util/execdetails/execdetails_test.go:1195:3: unknown field IaRemoteReadSegmentBytes in struct literal of type "github.com/tikv/client-go/v2/util".ScanDetail
pkg/util/execdetails/execdetails_test.go:1196:3: unknown field IaRemoteReadSegmentDuration in struct literal of type "github.com/tikv/client-go/v2/util".ScanDetail
(typecheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/util/execdetails/execdetails.go` around lines 213 - 231, The new IA
remote-read fields are not available in the current client-go version, so the
added accessor in GetIARemoteReadSegmentStats will not compile yet. Hold this
change until the github.com/tikv/client-go/v2 bump lands, or guard/remove the
references to util.ScanDetail.IaRemoteReadSegmentCount,
IaRemoteReadSegmentBytes, and IaRemoteReadSegmentDuration so this package and
its tests only use fields that exist in the current dependency.
Source: Linters/SAST tools
| sumIAReadSegmentCount uint64 | ||
| maxIAReadSegmentCount uint64 | ||
| sumIARemoteReadSegmentSize uint64 | ||
| maxIARemoteReadSegmentSize uint64 | ||
| sumIARemoteReadSegmentWaitTime time.Duration | ||
| maxIARemoteReadSegmentWaitTime time.Duration |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Inconsistent naming: "IAReadSegment" vs "IARemoteReadSegment".
sumIAReadSegmentCount/maxIAReadSegmentCount drop the "Remote" that appears in the sibling fields (sumIARemoteReadSegmentSize, sumIARemoteReadSegmentWaitTime) and in the upstream source field itself (scanDetail.IaRemoteReadSegmentCount, per GetIARemoteReadSegmentStats in execdetails.go). This inconsistency propagates to the exported, permanent SQL column name in reader.go (AVG_IA_READ_SEGMENT_COUNT vs AVG_IA_REMOTE_READ_SEGMENT_SIZE/_WAIT_TIME). Since these become public INFORMATION_SCHEMA.STATEMENTS_SUMMARY column names, it's worth fixing the naming now before it ships as a permanent (and inconsistent) public API surface.
✏️ Suggested rename
- sumIAReadSegmentCount uint64
- maxIAReadSegmentCount uint64
+ sumIARemoteReadSegmentCount uint64
+ maxIARemoteReadSegmentCount uint64📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sumIAReadSegmentCount uint64 | |
| maxIAReadSegmentCount uint64 | |
| sumIARemoteReadSegmentSize uint64 | |
| maxIARemoteReadSegmentSize uint64 | |
| sumIARemoteReadSegmentWaitTime time.Duration | |
| maxIARemoteReadSegmentWaitTime time.Duration | |
| sumIARemoteReadSegmentCount uint64 | |
| maxIARemoteReadSegmentCount uint64 | |
| sumIARemoteReadSegmentSize uint64 | |
| maxIARemoteReadSegmentSize uint64 | |
| sumIARemoteReadSegmentWaitTime time.Duration | |
| maxIARemoteReadSegmentWaitTime time.Duration |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/util/stmtsummary/statement_summary.go` around lines 202 - 207, The field
naming is inconsistent across the statement summary stats, where
sumIAReadSegmentCount/maxIAReadSegmentCount omit “Remote” while the related
fields and upstream source use IARemoteReadSegment/ IaRemoteReadSegmentCount.
Rename the struct fields in statement_summary.go and update all matching
references in reader.go and tables.go/v2 so the exported SQL column becomes
AVG_IA_REMOTE_READ_SEGMENT_COUNT (and related names stay aligned). Use the
existing sibling symbols like sumIARemoteReadSegmentSize,
maxIARemoteReadSegmentWaitTime, and GetIARemoteReadSegmentStats as the naming
reference when updating the public INFORMATION_SCHEMA.STATEMENTS_SUMMARY
columns.
1 participant
What problem does this PR solve?
Issue Number: ref #69624
Problem Summary:
TiDB does not currently expose IA remote-read scan detail fields in its own observability surfaces, so operators cannot inspect remote segment count, bytes, or wait time through slow logs, statement summary, or Prometheus metrics.
What changed and how does it work?
Adds TiDB observability plumbing for IA remote-read scan details:
INFORMATION_SCHEMA.SLOW_QUERYand statement summary columnsThis intentionally reads the new client-go
util.ScanDetailfields directly instead of using reflection compatibility code.This PR depends on
tikv/client-go#2015so TiDB can bumpgithub.com/tikv/client-go/v2to a version containingIaRemoteReadSegment*fields. The PR intentionally does not commit a temporary fork or localreplace.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
New Features
Bug Fixes