*: add tidb_paging_size_bytes and forward it to coprocessor RPC#68091
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:
📝 WalkthroughWalkthroughThe PR refreshes the Changeskvproto pin refresh
Paging byte-budget coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
ti-chi-bot
Bot
added
release-note-none
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/store/copr/coprocessor.go (1)
124-151:⚠️ Potential issue | 🟡 MinorKeep
DisablePagingfrom being bypassed by byte-budget paging.Line 124 only turns off
req.Paging.Enable, but Lines 144-150 can immediately turn paging back on whenPagingSizeBytes > 0. That makes the failpoint stop disabling paging for the new path, which can break failpoint-based tests and debugging.Proposed fix
failpoint.Inject("DisablePaging", func(_ failpoint.Value) { req.Paging.Enable = false + req.Paging.PagingSizeBytes = 0 })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/copr/coprocessor.go` around lines 124 - 151, The DisablePaging failpoint only clears req.Paging.Enable, but later byte-budget logic (pagingBytesEligible + req.Paging.PagingSizeBytes > 0) can re-enable paging; update the failpoint handler to also clear the byte-budget signal by setting req.Paging.PagingSizeBytes = 0 (and keep req.Paging.Enable = false) so the later block cannot force paging back on; locate the failpoint.Inject("DisablePaging", ...) and modify its closure to zero out req.Paging.PagingSizeBytes (and optionally other paging fields you want forced off) to ensure the failpoint fully disables paging.
🧹 Nitpick comments (1)
go.mod (1)
370-373: Consider adding explicit cleanup tracking for the temporary kvproto fork pin.This temporary replace is appropriate while PR pingcap/kvproto#1448 remains unmerged. However, tying the cleanup to a concrete tracker (issue link or internal checklist) would improve maintainability and prevent long-term drift on the personal fork.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 370 - 373, The temporary go.mod replace pin for github.com/pingcap/kvproto => github.com/JmPotato/kvproto v0.0.0-20260428063603-042bfc75f2ac needs explicit cleanup tracking: add a short TODO comment next to that replace line referencing a concrete tracker (create a public issue or internal checklist ID and paste the URL/ID) and include the upstream PR number (pingcap/kvproto#1448) and a deadline or review date; alternatively create the issue now and update the go.mod comment to point to that issue so reviewers can find and remove the fork pin when the upstream PR is merged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/sessionctx/variable/sysvar.go`:
- Around line 2985-2988: The new sysvar registration for TiDBPagingSizeBytes
omits the SET_VAR hint eligibility flag; update the var entry for Name:
vardef.TiDBPagingSizeBytes to include IsHintUpdatableVerified: true alongside
the existing Scope/Name/Value/Type/MinValue/MaxValue/SetSession fields so the
variable is explicitly marked eligible for SET_VAR hint handling (keep the
current SetSession closure as-is).
---
Outside diff comments:
In `@pkg/store/copr/coprocessor.go`:
- Around line 124-151: The DisablePaging failpoint only clears
req.Paging.Enable, but later byte-budget logic (pagingBytesEligible +
req.Paging.PagingSizeBytes > 0) can re-enable paging; update the failpoint
handler to also clear the byte-budget signal by setting
req.Paging.PagingSizeBytes = 0 (and keep req.Paging.Enable = false) so the later
block cannot force paging back on; locate the failpoint.Inject("DisablePaging",
...) and modify its closure to zero out req.Paging.PagingSizeBytes (and
optionally other paging fields you want forced off) to ensure the failpoint
fully disables paging.
---
Nitpick comments:
In `@go.mod`:
- Around line 370-373: The temporary go.mod replace pin for
github.com/pingcap/kvproto => github.com/JmPotato/kvproto
v0.0.0-20260428063603-042bfc75f2ac needs explicit cleanup tracking: add a short
TODO comment next to that replace line referencing a concrete tracker (create a
public issue or internal checklist ID and paste the URL/ID) and include the
upstream PR number (pingcap/kvproto#1448) and a deadline or review date;
alternatively create the issue now and update the go.mod comment to point to
that issue so reviewers can find and remove the fork pin when the upstream PR is
merged.
🪄 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: 82e5a82a-02bd-465b-826d-ac92d5da7b83
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
DEPS.bzlgo.modpkg/distsql/context/context.gopkg/distsql/context/context_test.gopkg/distsql/request_builder.gopkg/kv/kv.gopkg/session/session.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_test.gotests/integrationtest/r/sessionctx/setvar.resulttests/integrationtest/t/sessionctx/setvar.test
| {Scope: vardef.ScopeGlobal | vardef.ScopeSession, Name: vardef.TiDBPagingSizeBytes, Value: strconv.Itoa(vardef.DefPagingSizeBytes), Type: vardef.TypeUnsigned, MinValue: 0, MaxValue: math.MaxInt64, SetSession: func(s *SessionVars, val string) error { | ||
| s.PagingSizeBytes = int(TidbOptInt64(val, int64(vardef.DefPagingSizeBytes))) | ||
| return nil | ||
| }}, |
There was a problem hiding this comment.
Add explicit SET_VAR-hint eligibility metadata for tidb_paging_size_bytes.
Line 2985 registers the new sysvar but omits IsHintUpdatableVerified: true. Since this variable is intended to be used in SET_VAR(...), please mark it explicitly to avoid hint gating mismatches.
🔧 Proposed fix
-{Scope: vardef.ScopeGlobal | vardef.ScopeSession, Name: vardef.TiDBPagingSizeBytes, Value: strconv.Itoa(vardef.DefPagingSizeBytes), Type: vardef.TypeUnsigned, MinValue: 0, MaxValue: math.MaxInt64, SetSession: func(s *SessionVars, val string) error {
+{Scope: vardef.ScopeGlobal | vardef.ScopeSession, Name: vardef.TiDBPagingSizeBytes, Value: strconv.Itoa(vardef.DefPagingSizeBytes), Type: vardef.TypeUnsigned, MinValue: 0, MaxValue: math.MaxInt64, IsHintUpdatableVerified: true, SetSession: func(s *SessionVars, val string) error {📝 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.
| {Scope: vardef.ScopeGlobal | vardef.ScopeSession, Name: vardef.TiDBPagingSizeBytes, Value: strconv.Itoa(vardef.DefPagingSizeBytes), Type: vardef.TypeUnsigned, MinValue: 0, MaxValue: math.MaxInt64, SetSession: func(s *SessionVars, val string) error { | |
| s.PagingSizeBytes = int(TidbOptInt64(val, int64(vardef.DefPagingSizeBytes))) | |
| return nil | |
| }}, | |
| {Scope: vardef.ScopeGlobal | vardef.ScopeSession, Name: vardef.TiDBPagingSizeBytes, Value: strconv.Itoa(vardef.DefPagingSizeBytes), Type: vardef.TypeUnsigned, MinValue: 0, MaxValue: math.MaxInt64, IsHintUpdatableVerified: true, SetSession: func(s *SessionVars, val string) error { | |
| s.PagingSizeBytes = int(TidbOptInt64(val, int64(vardef.DefPagingSizeBytes))) | |
| return nil | |
| }}, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/sessionctx/variable/sysvar.go` around lines 2985 - 2988, The new sysvar
registration for TiDBPagingSizeBytes omits the SET_VAR hint eligibility flag;
update the var entry for Name: vardef.TiDBPagingSizeBytes to include
IsHintUpdatableVerified: true alongside the existing
Scope/Name/Value/Type/MinValue/MaxValue/SetSession fields so the variable is
explicitly marked eligible for SET_VAR hint handling (keep the current
SetSession closure as-is).
|
@JmPotato: The following test 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. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #68091 +/- ##
================================================
- Coverage 76.3268% 74.4628% -1.8641%
================================================
Files 2041 2045 +4
Lines 561003 573862 +12859
================================================
- Hits 428196 427314 -882
- Misses 131906 146359 +14453
+ Partials 901 189 -712
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| if pagingBytesEligible(req) && req.Paging.PagingSizeBytes > 0 { | ||
| if !req.Paging.Enable { | ||
| req.Paging.Enable = true | ||
| req.Paging.MinPagingSize = paging.MinPagingSize |
There was a problem hiding this comment.
PagingSizeBytes already bounds each page, so the geometric ramp-up serves no purpose here. Since the user set tidb_enable_paging = OFF, so we shouldn't reintroduce what they disabled. Suggest setting it to the same value as MaxPagingSize.
| // The TiKV coprocessor protocol requires paging to be enabled for the | ||
| // PagingSizeBytes field to take effect. When force-enabling paging, we | ||
| // use minimal row-count parameters so the byte budget becomes the | ||
| // dominant page-break signal. |
There was a problem hiding this comment.
To make the byte budget dominant, Min/MaxPagingSize must be large (so row-count cap doesn't fire first), not minimal.
With this being said, we might want to set Min/MaxPagingSize (line 147/148) to larger values. The math here is: For a 4 MiB byte budget, current MaxPagingSize setup means the byte budget only dominates when row_width ≥ 84 B.
JmPotato
force-pushed
the
paging-size-bytes
branch
from
48efbe7 to
852f963
Compare
ti-chi-bot
Bot
added
size/L
JmPotato
force-pushed
the
paging-size-bytes
branch
from
852f963 to
97de07a
Compare
ti-chi-bot
Bot
added
size/XL
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/mirror/BUILD.bazel (1)
36-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop
flaky = Trueunless this target is known to be nondeterministic.These tests only exercise local parsing/helpers, so marking the target flaky will mask real regressions and add noisy reruns in CI. As per coding guidelines, "Test files: Prefer extending existing test suites and fixtures over creating new scaffolding; keep test changes minimal and deterministic."
Suggested change
go_test( name = "mirror_test", timeout = "short", srcs = ["skylarkutil_test.go"], embed = [":mirror_lib"], - flaky = True, shard_count = 2, )🤖 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 `@cmd/mirror/BUILD.bazel` around lines 36 - 43, Remove the flaky setting from the mirror_test go_test target in BUILD.bazel unless there is a proven nondeterminism issue; keep the test definition for mirror_test deterministic and minimal by deleting the flaky attribute and leaving the rest of the target unchanged.Source: Coding guidelines
cmd/mirror/mirror.go (1)
397-400: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap proxy-URL formatting failures with module coordinates.
A bare
return errhere makes it hard to tell which dependency broke generation. Includereplaced.Pathandreplaced.Versionin the error so failures are actionable. As per coding guidelines, "Go code: Keep error handling actionable and contextual; avoid silently swallowing errors."Suggested change
goProxyURL, err := formatGoProxyURL(replaced.Path, replaced.Version) if err != nil { - return err + return fmt.Errorf("format go proxy url for %s@%s: %w", replaced.Path, replaced.Version, 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 `@cmd/mirror/mirror.go` around lines 397 - 400, The error returned from formatGoProxyURL in mirror.go is too bare and loses module context. Update the error handling in the proxy URL generation flow to wrap the returned error with the module coordinates from replaced.Path and replaced.Version so the failing dependency is identifiable. Use the existing variables in this block around formatGoProxyURL to add actionable context before returning the error.Source: Coding guidelines
🤖 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 `@cmd/mirror/BUILD.bazel`:
- Around line 36-43: Remove the flaky setting from the mirror_test go_test
target in BUILD.bazel unless there is a proven nondeterminism issue; keep the
test definition for mirror_test deterministic and minimal by deleting the flaky
attribute and leaving the rest of the target unchanged.
In `@cmd/mirror/mirror.go`:
- Around line 397-400: The error returned from formatGoProxyURL in mirror.go is
too bare and loses module context. Update the error handling in the proxy URL
generation flow to wrap the returned error with the module coordinates from
replaced.Path and replaced.Version so the failing dependency is identifiable.
Use the existing variables in this block around formatGoProxyURL to add
actionable context before returning the error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d473fd72-4dec-42c6-b15d-4bad646032b9
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
DEPS.bzlcmd/mirror/BUILD.bazelcmd/mirror/mirror.gocmd/mirror/skylarkutil.gocmd/mirror/skylarkutil_test.gogo.modpkg/distsql/BUILD.bazelpkg/distsql/request_builder_test.gopkg/store/copr/BUILD.bazelpkg/store/copr/copr_test/coprocessor_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- DEPS.bzl
JmPotato
force-pushed
the
paging-size-bytes
branch
3 times, most recently
from
1d8aa66 to
7bffdb2
Compare
ti-chi-bot
Bot
added
size/L
JmPotato
force-pushed
the
paging-size-bytes
branch
3 times, most recently
from
a349ee6 to
358f697
Compare
| req.Paging.Enable = false | ||
| } | ||
| var pagingSizeBytes uint64 | ||
| if req.StoreType == kv.TiKV && req.Tp == kv.ReqTypeDAG && req.Paging.PagingSizeBytes > 0 { |
There was a problem hiding this comment.
The DisablePaging failpoint above can still send byte-budget paging because this preserves PagingSizeBytes. Please clear PagingSizeBytes in the failpoint too.
There was a problem hiding this comment.
Done — the DisablePaging failpoint now clears PagingSizeBytes alongside Enable:
failpoint.Inject("DisablePaging", func(_ failpoint.Value) {
req.Paging.Enable = false
req.Paging.PagingSizeBytes = 0
})As extra hardening, BuildCopIterator now also zeroes req.Paging.PagingSizeBytes for any non-TiKV / non-DAG request, mirroring how Paging.Enable is handled. So req.Paging.PagingSizeBytes is the single source of truth and the byte budget can no longer slip past the gate from any path.
JmPotato
force-pushed
the
paging-size-bytes
branch
from
358f697 to
104d606
Compare
ti-chi-bot
Bot
removed
the
size/L
ti-chi-bot
Bot
added
the
size/XL
JmPotato
force-pushed
the
paging-size-bytes
branch
from
104d606 to
aaa0d75
Compare
| builder.SetPaging(dctx.EnablePaging) | ||
| builder.Request.Paging.MinPagingSize = uint64(dctx.MinPagingSize) | ||
| builder.Request.Paging.MaxPagingSize = uint64(dctx.MaxPagingSize) | ||
| builder.Request.Paging.PagingSizeBytes = uint64(dctx.PagingSizeBytes) |
There was a problem hiding this comment.
Non-blocking: byte-only paging has Enable == false, so downstream selectResult.paging will still be false and DistSQLQueryHistogram labels it as common. Could we include PagingSizeBytes > 0 there too?
There was a problem hiding this comment.
Good catch — done. selectResult.paging now includes the byte budget, so byte-only paging is recorded as paging (not common) in DistSQLQueryHistogram:
paging: kvReq.Paging.Enable || kvReq.Paging.PagingSizeBytes > 0,That field is observability-only (its only use is the histogram label), so this is purely a metrics-accuracy fix with no execution impact.
JmPotato
force-pushed
the
paging-size-bytes
branch
2 times, most recently
from
b1d701a to
2374ae7
Compare
| // in paging request, a request will be returned in multi batches, | ||
| // enlarge the channel size to avoid the request blocked by buffer full. | ||
| if req.Paging.Enable { | ||
| if req.Paging.Enable || req.Paging.PagingSizeBytes > 0 { |
There was a problem hiding this comment.
The PagingSizeBytes would enable coprocessor paging execution even the Paging is disabled, not sure if it is a possible risk.
Previously, there would be performance regression in some cases when paging is used.
There was a problem hiding this comment.
This is by-design behavior.
The req.Paging.Enable switch was used only to control whether paging_size was enabled. However, paging_size_bytes follows a completely different logic path. Although the names are similar, paging_size_bytes is controlled exclusively by RU-based Resource Control.
In the current implementation, beyond gating the Resource Control feature itself, we will introduce even stricter logic. Specifically, paging_size_bytes will only be enabled when Resource Group is active and there is significant throttling or a clear shortage of tokens.
The goal is to achieve more granular control rather than enabling it across all scenarios. Therefore, we chose not to couple it with the req.Paging.Enable switch and instead use a relatively independent logic.
There was a problem hiding this comment.
Got it, better keeping an eye on the performance regression tests.
There was a problem hiding this comment.
We will conduct dedicated regression testing to ensure that enabling paging_size_bytes does not impact the performance of normal workloads.
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
Add a TiDB-side knob `tidb_paging_size_bytes` (global+session sysvar, default 0) and plumb the value end-to-end so it lands as the new `paging_size_bytes` field on `coprocessor.Request`. - `tidb_paging_size_bytes` is allowed in `SET_VAR` hints; default 0 disables byte-budget paging. - The value flows through `DistSQLContext` into `kv.Request.Paging.PagingSizeBytes` via `RequestBuilder`. - In the cop client, `pagingBytesEligible` gates the feature to TiKV + DAG requests. When the byte budget is positive on an eligible request whose row-count paging is disabled, paging is force-enabled with minimum row parameters so the byte budget becomes the dominant page-break signal. This must happen before `checkStoreBatchCopr` because batch copr is incompatible with paging. - The byte budget is carried on each generated `copTask`, populates `coprocessor.Request.PagingSizeBytes` on outgoing RPCs, propagates onto retried tasks (region/lock errors), and is cleared together with `pagingSize` when the small-limit downgrade fires. Dependency: - Pin `pingcap/kvproto` to JmPotato/kvproto demo/ru-paging-size which adds `paging_size_bytes` (tag 17) to `coprocessor.Request` via pingcap/kvproto#1448; revert this replace once that PR is merged and tagged. Tests: - `TestPagingBytesEligible` covers eligibility for TiKV/DAG vs TiFlash and non-DAG requests. - `TestBuildCopTasksWithPagingSizeBytes` covers task construction with and without small-limit downgrade. - `tests/integrationtest/sessionctx/setvar` is extended to cover the new sysvar (default, set, set via SET_VAR hint). Signed-off-by: JmPotato <github@ipotato.me>
Byte-budget paging only delivers RU-control value for resource groups that are hard-capped at RU_PER_SEC. For burstable or unlimited groups, or when Resource Control is disabled, the per-page byte break adds RPC overhead without bounding any token budget. Add an `RCNonBurstable` bit to `kv.Request.Paging` and propagate it through `DistSQLContext`. The bit is computed once per `DistSQLContext` in `session.GetDistSQLCtx` from the current statement's resource group: it is true iff `EnableResourceControl` is on and `InfoSchema.ResourceGroupByName(...)` returns a group whose `GetBurstLimitAdjusted()` is non-negative. The default group is treated as a normal group: by default its `BurstLimit` is -1 so byte-budget paging stays off, but if the user alters its config to a non-negative `BurstLimit` it will trigger like any other RC-capped group. `pagingBytesEligible` now requires `req.Paging.RCNonBurstable` in addition to TiKV + DAG, so the byte budget is only set on RPCs that will benefit from it. Signed-off-by: JmPotato <github@ipotato.me>
Signed-off-by: JmPotato <github@ipotato.me>
Signed-off-by: JmPotato <github@ipotato.me>
Signed-off-by: JmPotato <github@ipotato.me>
Drop the redundant per-task copTask.pagingSizeBytes and the buildCopTaskOpt field, and read req.Paging.PagingSizeBytes directly, mirroring how Paging.Enable is gated. BuildCopIterator now zeroes the byte budget for non-TiKV/non-DAG requests, which also removes a stray budget that could spuriously disable store-batch copr on TiFlash or non-DAG requests. New retry/split paths can no longer drop the budget. Simplify the cop cache key back to a single paging marker byte: a cached page is self-describing via its returned range, so the exact PagingSize / PagingSizeBytes values never affect cache correctness. This reverts the unnecessary 8-byte encodings and removes the latent PagingSize=N vs PagingSizeBytes=N key collision. Clear Paging.PagingSizeBytes in index-lookup push down, matching the existing Paging.Enable/Cacheable handling. Signed-off-by: JmPotato <github@ipotato.me>
JmPotato
force-pushed
the
paging-size-bytes
branch
from
2374ae7 to
78c69d1
Compare
yudongusa
left a comment
yudongusa
left a comment
There was a problem hiding this comment.
Please open a document PR to state this is an internal use variable
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, rleungx, yudongusa 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 |
|
/retest |
|
@JmPotato: The following test 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. |
5 participants
What problem does this PR solve?
Issue Number: close #68090
Problem Summary:
Today TiDB's coprocessor paging protocol breaks pages only on row count (
paging_size). For Resource Control workloads, RU pre-charge in PD's resource controller would benefit from an additional, byte-granular page-break signal so that a single page's response volume is bounded by bytes rather than only by rows.pingcap/kvproto#1448 adds a new
paging_size_bytesfield (tag 17) oncoprocessor.Request. This PR adds the TiDB-side knob and plumbing so the byte budget can be set per session and forwarded onto each coprocessor RPC.What changed and how does it work?
tidb_paging_size_bytesglobal+session sysvar (default0, disabled). Allowed insideSET_VARhints.session.GetDistSQLCtxresolves the effective byte budget once: it keeps the session value only when Resource Control is enabled and the statement resource group has a non-negative adjusted burst limit; otherwise it forwards0.DistSQLContext.PagingSizeBytesintokv.Request.Paging.PagingSizeBytesviaRequestBuilder; no extra RC state is carried inkv.Request.pkg/store/copr:BuildCopIteratorzeroes the budget for any other request (non-TiKV or non-DAG), mirroring howPaging.Enableis handled, sokv.Request.Paging.PagingSizeBytesstays the single source of truth.paging_size_bytesis propagated independently from row-countpaging_size; it does not force-enable or rewrite row-count paging settings, and it survives row-count small-limit downgrades.coprocessor.Request.PagingSizeBytes, retries, and region splits) take the budget straight off the request instead of a per-task copy, so no retry/split path can drop it. Index-lookup push down clears the budget alongsidePaging.Enable/Cacheable.paging_size/paging_size_bytesvalues never affect cache correctness, and paging vs non-paging requests stay in separate key spaces (no collision).Check List
Tests
Validated locally:
./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 -run 'Test(BuildCacheKey|GetSet|BuildCopTasksWithPagingSizeBytes)$' -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)make lintgit diff --checkDocumentation
Related PRs
Release note
Summary by CodeRabbit
New Features
Bug Fixes