Repository navigation
*: backport RC paging pre-charge to release-nextgen-202603 - #69830
Merged
ti-chi-bot[bot] merged 4 commits intoJul 20, 2026
Conversation
ti-chi-bot
Bot
added
release-note
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesPaging configuration and propagation
Coprocessor byte-budget execution
Paging pre-charge dashboards
Dependency metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Session
participant DistSQL
participant Coprocessor
participant TiKV
Session->>DistSQL: Set PagingSizeBytes
DistSQL->>Coprocessor: Build byte-budget request
Coprocessor->>TiKV: Send predicted read bytes
TiKV-->>Coprocessor: Return page and observed read bytes
Coprocessor->>Coprocessor: Update EMA
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/store/copr/ema.go (1)
37-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid relying on implicit
time.Subclamping and extremefloat64bounds.The current design leaves
lastObsAtas the zero time, relying onnow.Sub(zeroTime)to overflow and be clamped by Go'stimepackage tomath.MaxInt64. This results inmath.Exp(-huge)underflowing to0.0, yielding analphaof exactly1.0to seamlessly replace the seed value.While functionally correct and clever, relying on edge-case arithmetic and implicit bounds clamping is non-obvious and brittle. Using an explicit
IsZero()check achieves the exact same result while clarifying the intent for future maintainers.♻️ Proposed refactor
-func newRUEMA(seedReadBytes uint64) *ruEMA { - // lastObsAt is intentionally left as the zero time. If there is no seed, - // the first Observe gets alpha ~= 1 and seeds value from the first sample; - // if there is a byte-budget seed, the first real sample replaces it. - return &ruEMA{tau: defaultRUEMATau, value: float64(seedReadBytes)} -} - -func (e *ruEMA) Observe(bytes uint64, now time.Time) { - e.mu.Lock() - defer e.mu.Unlock() - dt := now.Sub(e.lastObsAt) - if dt < 0 { - dt = 0 - } - alpha := 1 - math.Exp(-float64(dt)/float64(e.tau)) - e.value += alpha * (float64(bytes) - e.value) - // Don't rewind on out-of-order Observes. - if now.After(e.lastObsAt) { - e.lastObsAt = now - } -} +func newRUEMA(seedReadBytes uint64) *ruEMA { + // If there is a byte-budget seed, the first real sample replaces it. + return &ruEMA{tau: defaultRUEMATau, value: float64(seedReadBytes)} +} + +func (e *ruEMA) Observe(bytes uint64, now time.Time) { + e.mu.Lock() + defer e.mu.Unlock() + + if e.lastObsAt.IsZero() { + e.value = float64(bytes) + e.lastObsAt = now + return + } + + dt := now.Sub(e.lastObsAt) + if dt < 0 { + dt = 0 + } + alpha := 1 - math.Exp(-float64(dt)/float64(e.tau)) + e.value += alpha * (float64(bytes) - e.value) + // Don't rewind on out-of-order Observes. + if now.After(e.lastObsAt) { + e.lastObsAt = now + } +}🤖 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/store/copr/ema.go` around lines 37 - 57, Update ruEMA.Observe to explicitly detect an unset e.lastObsAt with IsZero() and use alpha 1 for that first observation, avoiding time.Sub on the zero timestamp and extreme float64 calculations; preserve the existing non-zero timestamp decay and out-of-order timestamp behavior.
🤖 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.
Nitpick comments:
In `@pkg/store/copr/ema.go`:
- Around line 37-57: Update ruEMA.Observe to explicitly detect an unset
e.lastObsAt with IsZero() and use alpha 1 for that first observation, avoiding
time.Sub on the zero timestamp and extreme float64 calculations; preserve the
existing non-zero timestamp decay and out-of-order timestamp behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d0f82cb3-fcd6-4c1a-8297-ced7640fa15d
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (41)
DEPS.bzlDockerfileDockerfile.enterpriseWORKSPACEbuild/image/basebuild/image/parser_testgo.modpkg/distsql/BUILD.bazelpkg/distsql/context/context.gopkg/distsql/context/context_test.gopkg/distsql/distsql.gopkg/distsql/request_builder.gopkg/distsql/request_builder_test.gopkg/domain/infosync/BUILD.bazelpkg/domain/infosync/resource_manager_client.gopkg/executor/adapter_internal_test.gopkg/executor/distsql.gopkg/kv/kv.gopkg/metrics/grafana/tidb_resource_control.jsonpkg/metrics/grafana/tidb_resource_control.jsonnetpkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonpkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonnetpkg/session/session.gopkg/session/tidb_test.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/session.gopkg/sessionctx/variable/setvar_affect.gopkg/sessionctx/variable/sysvar.gopkg/store/copr/BUILD.bazelpkg/store/copr/batch_coprocessor.gopkg/store/copr/copr_test/coprocessor_test.gopkg/store/copr/coprocessor.gopkg/store/copr/coprocessor_cache.gopkg/store/copr/coprocessor_cache_test.gopkg/store/copr/coprocessor_test.gopkg/store/copr/ema.gopkg/store/copr/ema_test.gopkg/store/mockstore/unistore/pd.gopkg/store/mockstore/unistore/tikv/mock_region.gotests/integrationtest/r/sessionctx/setvar.resulttests/integrationtest/t/sessionctx/setvar.test
JmPotato
force-pushed
the
codex/rc-paging-precharge-release-nextgen-202603
branch
from
July 15, 2026 09:34
8177424 to
46cbdfe
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
pkg/store/mockstore/unistore/pd.go (1)
438-440: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
cmp.Compareover manual type conversion and subtraction.Using
cmp.Comparefrom the standardcmppackage is safer and more idiomatic. It prevents potential integer overflow issues on 32-bit architectures whereintis 32 bits, which could occur when subtracting largeuint32values.♻️ Proposed refactor
- index, exists := slices.BinarySearchFunc(m.keyspaces, id, func(k *keyspacepb.KeyspaceMeta, idToSearch uint32) int { - return int(k.Id) - int(idToSearch) - }) + index, exists := slices.BinarySearchFunc(m.keyspaces, id, func(k *keyspacepb.KeyspaceMeta, idToSearch uint32) int { + return cmp.Compare(k.Id, idToSearch) + })Note: Make sure to add
"cmp"to the imports if it is not already imported.🤖 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/store/mockstore/unistore/pd.go` around lines 438 - 440, Update the comparison function passed to slices.BinarySearchFunc in the keyspace lookup to use cmp.Compare with the uint32 keyspace ID and idToSearch values, avoiding manual int conversion and subtraction. Add the standard cmp import if needed.pkg/store/copr/coprocessor_test.go (1)
713-717: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
assert.NoErroroverrequire.NoErrorinsidedefer.Using
require.NoErrorinside adeferis unsafe because it callst.FailNow()(which triggersruntime.Goexit()) on failure. If the test is already panicking or failing, this will abruptly terminate the goroutine and swallow the original panic or error trace, making debugging difficult. Consider usingassert.NoErrorinstead.♻️ Proposed refactor
defer func() { pdClient.Close() err = mockClient.Close() - require.NoError(t, err) + assert.NoError(t, err) }()🤖 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/store/copr/coprocessor_test.go` around lines 713 - 717, In the deferred cleanup block, replace require.NoError with assert.NoError when checking mockClient.Close(). Keep the existing pdClient.Close() cleanup and error assignment unchanged, ensuring cleanup failures do not terminate the test goroutine or mask the original failure.pkg/store/copr/ema_test.go (1)
124-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest both configurations if
NextGenis mutable.This assertion only tests the behavior corresponding to the current global state of
clientgoconfig.NextGen. Consequently, the inactive branch is left untested in a given test run.If
NextGenis a mutable variable, consider toggling its state to explicitly guarantee coverage for both the classic and next-gen pathways.💡 Proposed refactor to test both code paths
- want := tt.classicWant - if clientgoconfig.NextGen { - want = tt.nextGenWant - } - require.Equal(t, want, pagingResponseReadBytes(resp)) + // Save the original state and restore it after testing. + originalNextGen := clientgoconfig.NextGen + defer func() { clientgoconfig.NextGen = originalNextGen }() + + // Test classic behavior + clientgoconfig.NextGen = false + require.Equal(t, tt.classicWant, pagingResponseReadBytes(resp)) + + // Test NextGen behavior + clientgoconfig.NextGen = true + require.Equal(t, tt.nextGenWant, pagingResponseReadBytes(resp))🤖 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/store/copr/ema_test.go` around lines 124 - 128, Update the test around pagingResponseReadBytes to explicitly exercise both clientgoconfig.NextGen states rather than selecting one expected value from the current global state. Toggle NextGen for separate classic and next-gen assertions, restoring the original state afterward, and use classicWant and nextGenWant respectively.pkg/store/copr/ema.go (1)
37-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an explicit
IsZerocheck for the first observation.Relying on
now.Sub(time.Time{})to produce a massivedtand yieldalpha ≈ 1is an elegant trick, but it can be fragile. If a unit test environment mocks time to be near the Unix epoch, the calculateddtwill be smaller,alphawill not equal1.0, and the seed value will unexpectedly leak into the first real sample's calculation.Consider explicitly checking for the uninitialized state to make the code robust against any time value.
💡 Proposed refactor to explicitly handle the first observation
func newRUEMA(seedReadBytes uint64) *ruEMA { - // lastObsAt is intentionally left as the zero time. If there is no seed, - // the first Observe gets alpha ~= 1 and seeds value from the first sample; - // if there is a byte-budget seed, the first real sample replaces it. + // lastObsAt is intentionally left as the zero time to signal the first + // Observe to completely replace the initial seed value with the first sample. return &ruEMA{tau: defaultRUEMATau, value: float64(seedReadBytes)} } func (e *ruEMA) Observe(bytes uint64, now time.Time) { e.mu.Lock() defer e.mu.Unlock() + + if e.lastObsAt.IsZero() { + e.value = float64(bytes) + e.lastObsAt = now + return + } + dt := now.Sub(e.lastObsAt) if dt < 0 { dt = 0 } alpha := 1 - math.Exp(-float64(dt)/float64(e.tau)) e.value += alpha * (float64(bytes) - e.value) // Don't rewind on out-of-order Observes. if now.After(e.lastObsAt) { e.lastObsAt = now } }🤖 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/store/copr/ema.go` around lines 37 - 57, Update ruEMA.Observe to explicitly detect an uninitialized lastObsAt with IsZero before calculating dt and alpha; on the first observation, replace the seeded value with the sample directly and initialize lastObsAt, while preserving the existing smoothing and out-of-order timestamp behavior for subsequent observations.
🤖 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.
Nitpick comments:
In `@pkg/store/copr/coprocessor_test.go`:
- Around line 713-717: In the deferred cleanup block, replace require.NoError
with assert.NoError when checking mockClient.Close(). Keep the existing
pdClient.Close() cleanup and error assignment unchanged, ensuring cleanup
failures do not terminate the test goroutine or mask the original failure.
In `@pkg/store/copr/ema_test.go`:
- Around line 124-128: Update the test around pagingResponseReadBytes to
explicitly exercise both clientgoconfig.NextGen states rather than selecting one
expected value from the current global state. Toggle NextGen for separate
classic and next-gen assertions, restoring the original state afterward, and use
classicWant and nextGenWant respectively.
In `@pkg/store/copr/ema.go`:
- Around line 37-57: Update ruEMA.Observe to explicitly detect an uninitialized
lastObsAt with IsZero before calculating dt and alpha; on the first observation,
replace the seeded value with the sample directly and initialize lastObsAt,
while preserving the existing smoothing and out-of-order timestamp behavior for
subsequent observations.
In `@pkg/store/mockstore/unistore/pd.go`:
- Around line 438-440: Update the comparison function passed to
slices.BinarySearchFunc in the keyspace lookup to use cmp.Compare with the
uint32 keyspace ID and idToSearch values, avoiding manual int conversion and
subtraction. Add the standard cmp import if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6574ebbd-d795-4125-b23a-040686408835
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (41)
DEPS.bzlDockerfileDockerfile.enterpriseWORKSPACEbuild/image/basebuild/image/parser_testgo.modpkg/distsql/BUILD.bazelpkg/distsql/context/context.gopkg/distsql/context/context_test.gopkg/distsql/distsql.gopkg/distsql/request_builder.gopkg/distsql/request_builder_test.gopkg/domain/infosync/BUILD.bazelpkg/domain/infosync/resource_manager_client.gopkg/executor/adapter_internal_test.gopkg/executor/distsql.gopkg/kv/kv.gopkg/metrics/grafana/tidb_resource_control.jsonpkg/metrics/grafana/tidb_resource_control.jsonnetpkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonpkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonnetpkg/session/session.gopkg/session/tidb_test.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/session.gopkg/sessionctx/variable/setvar_affect.gopkg/sessionctx/variable/sysvar.gopkg/store/copr/BUILD.bazelpkg/store/copr/batch_coprocessor.gopkg/store/copr/copr_test/coprocessor_test.gopkg/store/copr/coprocessor.gopkg/store/copr/coprocessor_cache.gopkg/store/copr/coprocessor_cache_test.gopkg/store/copr/coprocessor_test.gopkg/store/copr/ema.gopkg/store/copr/ema_test.gopkg/store/mockstore/unistore/pd.gopkg/store/mockstore/unistore/tikv/mock_region.gotests/integrationtest/r/sessionctx/setvar.resulttests/integrationtest/t/sessionctx/setvar.test
🚧 Files skipped from review as they are similar to previous changes (30)
- Dockerfile.enterprise
- pkg/distsql/BUILD.bazel
- pkg/distsql/context/context_test.go
- build/image/base
- build/image/parser_test
- Dockerfile
- tests/integrationtest/r/sessionctx/setvar.result
- WORKSPACE
- pkg/store/copr/BUILD.bazel
- pkg/sessionctx/vardef/tidb_vars.go
- pkg/store/mockstore/unistore/tikv/mock_region.go
- tests/integrationtest/t/sessionctx/setvar.test
- pkg/sessionctx/variable/setvar_affect.go
- pkg/domain/infosync/BUILD.bazel
- pkg/sessionctx/variable/session.go
- pkg/sessionctx/variable/sysvar.go
- pkg/session/session.go
- pkg/distsql/request_builder.go
- pkg/kv/kv.go
- pkg/domain/infosync/resource_manager_client.go
- pkg/metrics/grafana/tidb_resource_control.json
- pkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonnet
- pkg/store/copr/coprocessor_cache.go
- pkg/metrics/grafana/tidb_resource_control.jsonnet
- pkg/store/copr/coprocessor_cache_test.go
- go.mod
- pkg/session/tidb_test.go
- pkg/distsql/request_builder_test.go
- pkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.json
- pkg/store/copr/coprocessor.go
3 of 13 tasks
JmPotato
force-pushed
the
codex/rc-paging-precharge-release-nextgen-202603
branch
from
July 16, 2026 11:53
46cbdfe to
85a15b9
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-nextgen-202603 #69830 +/- ##
===========================================================
Coverage ? 76.1812%
===========================================================
Files ? 1937
Lines ? 541091
Branches ? 0
===========================================================
Hits ? 412210
Misses ? 128881
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
JmPotato
force-pushed
the
codex/rc-paging-precharge-release-nextgen-202603
branch
from
July 16, 2026 13:32
85a15b9 to
0d73446
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/store/copr/ema_test.go (1)
124-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest both configurations for
pagingResponseReadBytes.The test currently only exercises one branch of the logic depending on the runtime environment's value of
clientgoconfig.NextGen. IfNextGenis a mutable global variable, consider iterating over both boolean states and overriding it during the test to ensure that both the classic and next-gen paths are deterministically verified in CI.💡 Proposed refactor
- want := tt.classicWant - if clientgoconfig.NextGen { - want = tt.nextGenWant - } - require.Equal(t, want, pagingResponseReadBytes(resp)) + for _, nextGen := range []bool{false, true} { + t.Run(fmt.Sprintf("NextGen=%v", nextGen), func(t *testing.T) { + originalNextGen := clientgoconfig.NextGen + clientgoconfig.NextGen = nextGen + t.Cleanup(func() { clientgoconfig.NextGen = originalNextGen }) + + want := tt.classicWant + if nextGen { + want = tt.nextGenWant + } + require.Equal(t, want, pagingResponseReadBytes(resp)) + }) + }🤖 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/store/copr/ema_test.go` around lines 124 - 128, Update the test around pagingResponseReadBytes to run deterministically with clientgoconfig.NextGen set to both false and true, selecting the matching classicWant or nextGenWant expectation for each case. Restore the original global value after the test, including when assertions fail, and preserve the existing response setup and equality checks.
🤖 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.
Nitpick comments:
In `@pkg/store/copr/ema_test.go`:
- Around line 124-128: Update the test around pagingResponseReadBytes to run
deterministically with clientgoconfig.NextGen set to both false and true,
selecting the matching classicWant or nextGenWant expectation for each case.
Restore the original global value after the test, including when assertions
fail, and preserve the existing response setup and equality checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 748689b6-6703-4bc8-bf60-d7d9512cb776
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (30)
DEPS.bzlgo.modpkg/distsql/BUILD.bazelpkg/distsql/context/context.gopkg/distsql/context/context_test.gopkg/distsql/distsql.gopkg/distsql/request_builder.gopkg/distsql/request_builder_test.gopkg/executor/distsql.gopkg/kv/kv.gopkg/metrics/grafana/tidb_resource_control.jsonpkg/metrics/grafana/tidb_resource_control.jsonnetpkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonpkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonnetpkg/session/session.gopkg/session/tidb_test.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/session.gopkg/sessionctx/variable/setvar_affect.gopkg/sessionctx/variable/sysvar.gopkg/store/copr/BUILD.bazelpkg/store/copr/copr_test/coprocessor_test.gopkg/store/copr/coprocessor.gopkg/store/copr/coprocessor_cache.gopkg/store/copr/coprocessor_cache_test.gopkg/store/copr/coprocessor_test.gopkg/store/copr/ema.gopkg/store/copr/ema_test.gotests/integrationtest/r/sessionctx/setvar.resulttests/integrationtest/t/sessionctx/setvar.test
🚧 Files skipped from review as they are similar to previous changes (20)
- pkg/distsql/BUILD.bazel
- pkg/sessionctx/variable/session.go
- tests/integrationtest/t/sessionctx/setvar.test
- pkg/distsql/context/context.go
- pkg/sessionctx/variable/setvar_affect.go
- pkg/sessionctx/variable/sysvar.go
- pkg/sessionctx/vardef/tidb_vars.go
- pkg/distsql/distsql.go
- pkg/store/copr/coprocessor_cache_test.go
- pkg/store/copr/copr_test/coprocessor_test.go
- pkg/distsql/request_builder.go
- pkg/session/session.go
- pkg/distsql/request_builder_test.go
- pkg/store/copr/coprocessor_test.go
- pkg/session/tidb_test.go
- pkg/metrics/grafana/tidb_resource_control.jsonnet
- pkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.json
- pkg/metrics/nextgengrafana/tidb_resource_control_with_keyspace_name.jsonnet
- pkg/store/copr/coprocessor_cache.go
- pkg/store/copr/coprocessor.go
YuhaoZhang00
approved these changes
Jul 17, 2026
|
@YuhaoZhang00: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this: Instructions 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. |
yudongusa
approved these changes
Jul 17, 2026
…cap#68091) close pingcap#68090 (cherry picked from commit 4ccb1ae) Signed-off-by: JmPotato <github@ipotato.me>
…ingcap#67941) close pingcap#68085 (cherry picked from commit ab7d93b) Signed-off-by: JmPotato <github@ipotato.me>
ref pingcap#68085 (cherry picked from commit 10292a4) Signed-off-by: JmPotato <github@ipotato.me>
JmPotato
force-pushed
the
codex/rc-paging-precharge-release-nextgen-202603
branch
from
July 18, 2026 09:42
0d73446 to
a380455
Compare
ref pingcap#68085 (cherry picked from commit cae12c1) Signed-off-by: JmPotato <github@ipotato.me>
JmPotato
force-pushed
the
codex/rc-paging-precharge-release-nextgen-202603
branch
from
July 18, 2026 10:22
a380455 to
6210e89
Compare
cfzjywxk
approved these changes
Jul 20, 2026
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
|
@nolouch-ai: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this:
Instructions 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. |
nolouch
approved these changes
Jul 20, 2026
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, nolouch, nolouch-ai, yudongusa, YuhaoZhang00 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
Bot
merged commit Jul 20, 2026
abf8c9f
into
pingcap:release-nextgen-202603
18 checks passed
6 of 13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: ref #68085
Problem Summary:
Backport RC paging pre-charge to
release-nextgen-202603. Without request-side pre-charge, concurrent paging coprocessor reads can consume a large amount of data before response settlement applies resource-control throttling. The paging gate must also use PD's effective runtime limited-burst state so service-limit overrides are not missed by static InfoSchema metadata. The earlier backport also left byte-budget paging disabled by default even for eligible non-burstable Resource Groups.What changed and how does it work?
tidb_paging_size_bytesand forward the effective byte budget to TiKV coprocessor requests for eligible resource groups.PredictedReadByteshint before dispatch.ResourceGroupRuntimeState.HasLimitedBurstwhen gatingtidb_paging_size_bytes, with InfoSchema as the fallback until runtime state is available.tidb_paging_size_bytesto 4 MiB for eligible non-burstable Resource Groups, declare its SET_VAR hint support inline, and document the tested performance rationale. Users can set it to0to disable byte-budget paging.Dependencies:
363178052f570892efff0d0048ce51eb8a8868fdonrelease-nextgen-2026031636e69a936bd4a3da008ba4cb66699899ce40fconrelease-nextgen-202603520a5635e5e39aecd9a3887ad6f2e38f46652c9aonrelease-nextgen-202603Source PRs:
Check List
Tests
Validated locally:
./tools/check/failpoint-go-test.sh pkg/store/copr -run TestRUEMA -count=1./tools/check/failpoint-go-test.sh pkg/store/copr -run TestPagingResponseReadBytes -count=1./tools/check/failpoint-go-test.sh pkg/store/copr -run TestBuildCopTasksWithPagingSizeBytes -count=1./tools/check/failpoint-go-test.sh pkg/domain/infosync -run TestPutBundlesRetry -count=1./tools/check/failpoint-go-test.sh pkg/store/mockstore/unistore/tikv -run TestMockGCStatesManager -count=1./tools/check/failpoint-go-test.sh pkg/session -run TestDistSQLCtxPagingSizeBytesRequiresHardCappedResourceGroup -count=1./tools/check/failpoint-go-test.sh pkg/distsql -run TestRequestBuilderKeepsPagingSizeBytesWhenPagingDisabled -count=1./tools/check/failpoint-go-test.sh pkg/store/copr/copr_test -run TestBuildCopIteratorWithBatchStoreCopr -count=1(cd tests/integrationtest && ./run-tests.sh -r sessionctx/setvar)(695 cases passed)make bazel_preparemake lintgit range-difffor each source/backport commit pairgit diff --check upstream/release-nextgen-202603...HEADSide effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
New Features
tidb_paging_size_bytessession variable.Bug Fixes