*: backport RC paging pre-charge to release-nextgen-202603#69830
Open
JmPotato wants to merge 3 commits into
Open
*: backport RC paging pre-charge to release-nextgen-202603#69830JmPotato wants to merge 3 commits into
JmPotato wants to merge 3 commits into
Conversation
ti-chi-bot
Bot
added
release-note
📝 WalkthroughWalkthroughAdds ChangesPaging pre-charge and repository refresh
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Session
participant DistSQL
participant Coprocessor
participant TiKV
Session->>DistSQL: propagate PagingSizeBytes
DistSQL->>Coprocessor: build paging request
Coprocessor->>TiKV: send predicted read bytes
TiKV-->>Coprocessor: return page and read-byte details
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
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
13 tasks
…cap#68091) close pingcap#68090 (cherry picked from commit 4ccb1ae) Signed-off-by: JmPotato <github@ipotato.me>
JmPotato
force-pushed
the
codex/rc-paging-precharge-release-nextgen-202603
branch
from
46cbdfe to
85a15b9
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-nextgen-202603 #69830 +/- ##
===========================================================
Coverage ? 76.1839%
===========================================================
Files ? 1937
Lines ? 541067
Branches ? 0
===========================================================
Hits ? 412206
Misses ? 128861
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…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
85a15b9 to
0d73446
Compare
|
[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 |
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
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
2 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.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.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
tidb_paging_size_bytessession variable;0disables byte-budget paging.