executor, store/copr: budget merged Analyze batches - #70003
Conversation
Capture the two boundaries that later commits must change or preserve: analyze requests do not yet join store batching, while batched tasks without a per-task response remain eligible for retry. Signed-off-by: 0xPoe <poe.liu@pm.me>
Forward tidb_store_batch_size to V2 full-sampling Analyze and admit those internal requests to store batching without row-count hints. Leave KeepOrder unset because the consumers derive correlation and other statistics from the collected samples rather than scan order. Signed-off-by: 0xPoe <poe.liu@pm.me>
Opt Analyze into kvproto batch-task data merging and consume explicit per-task acknowledgements without emitting empty responses. A task without an acknowledgement remains on the existing retry path, so mixed-version stores cannot cause silent statistics loss. Temporarily replace kvproto with the revision from pingcap/kvproto#1497 until that protocol change merges. Signed-off-by: 0xPoe <poe.liu@pm.me>
Fit full-sampling analyze requests into the configured scan concurrency: a batched group costs its physical width (children + 1), so shrink the outer iterator concurrency and cap children at TiKV's supported maximum (4) instead of exempting analyze from admission entirely. concurrency=1 degenerates to unbatched requests. Adopt the single-owner execution-details contract: the store's finalizer aggregates a merged child's details into the main response, which TiDB collects exactly once in handleCopResponse; stop re-collecting the per-task ACK copy, which would double count scan/time totals in runtime stats and runaway tracking. Depends on the TiKV finalizer aggregation landing in the same change set. Consolidate the RUEMA tests into subtests to keep pkg/store/copr within the toomanytests nogo limit (47/50). Signed-off-by: 0xTars <1196089730@qq.com>
Signed-off-by: 0xTars <1196089730@qq.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
Hi @0xTars. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
[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 |
|
|
Signed-off-by: 0xTars <1196089730@qq.com>
158af96 to
5a704a7
Compare
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the For example:
📖 For more info, you can check the "Contribute Code" section in the development guide. |
What problem does this PR solve?
Issue Number: ref #67449
Problem Summary:
Full-sampling Analyze batching could multiply physical scan and retained
collector concurrency by the batch width. Merged child execution details could
also be consumed twice: once from child acknowledgements and again from the
outer response.
This draft follows up on #69686 with the bounded-fanout and single-owner
accounting changes found during coordinated review.
What changed and how does it work?
allow_analyze_batch_task_data_merge.min(storeBatchSize + 1, analyzeConcurrency, 5).max(1, floor(analyzeConcurrency / width)), so the physical scan budgetnever exceeds the configured pre-batching Analyze concurrency.
once for runtime, runaway-query, and RU accounting.
coprocessor: scope batch merge capability to analyze kvproto#1504
coprocessor: specialize batched full-sampling Analyze finalization tikv/tikv#19865
Check List
Tests
Validation:
./tools/check/failpoint-go-test.sh pkg/executor -run '^TestAnalyzeBatchScanBudget$'./tools/check/failpoint-go-test.sh pkg/store/copr -run '^(TestHandleBatchCopResponseMergedAndUnansweredTasks|TestRUEMA)$'./tools/check/failpoint-go-test.sh pkg/store/copr -race -run '^TestRUEMA$'make bazel_preparemake lintgit diff --checkSide effects
Documentation
Release note