Skip to content

executor, store/copr: batch full-sampling Analyze requests - #70055

Draft
0xTars wants to merge 2 commits into
pingcap:masterfrom
0xTars:copper/analyze-batch-merge-v2
Draft

executor, store/copr: batch full-sampling Analyze requests#70055
0xTars wants to merge 2 commits into
pingcap:masterfrom
0xTars:copper/analyze-batch-merge-v2

Conversation

@0xTars

@0xTars 0xTars commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: ref #70059

Problem Summary:

V2 full-sampling ANALYZE normally sends one coprocessor request per TiKV
Region. Large physical tables can therefore create many small requests.
Same-store batching can reduce that RPC count, but the batching policy must use
the request's actual resolved Regions, preserve the configured Analyze
physical-scan budget, and keep retry and non-Analyze paths unchanged.

This replaces #69686 because its source branch cannot be updated. Issue #70059
tracks the cross-repository batching contract and landing gates.

What changed and how does it work?

  • Mark V2 full-sampling Analyze requests as able to consume merged child
    responses. The existing generic DAG path is unchanged.

  • Resolve all KeyRanges partitions for one Analyze kv.Request exactly once,
    deduplicate their Region IDs, select one request-local width, and pack each
    partition independently. Batches never cross stores or partition-builder
    boundaries, and descending response order is preserved.

  • Let N be the number of distinct Regions covered by one Analyze
    kv.Request (one nonpartitioned table or one current physical partition),
    and let C=max(resolved Analyze scan concurrency,1). The selected maximum
    same-store Region width is:

    if N <= 2*C:
        B = 1
    else:
        B = min(ceil(N/C), max { b | b*(b+1) <= C })
    

    The implementation uses overflow-safe integer comparisons. B=1 means
    singleton requests. The triangular cap guarantees
    floor(C/B) >= B+1 for a batched width.

  • Full-sampling Analyze does not use tidb_store_batch_size; generic DAG
    batching still does.

  • Derive iterator concurrency from actual built task weights
    (1 + child count) with a dynamic weight histogram. The admitted heaviest
    prefix never exceeds C, so the scalar worker count is safe for every
    scheduler order and for widths greater than five.

  • Apply the request-local policy only during the initial build. Region-error
    and lock retries rebuild singleton tasks. Fallback counters count each
    original failed child once even when one child splits into several retry
    tasks.

  • Keep unsigned integer-PK full-sampling Analyze in one request across
    MaxInt64. The sampler reconstructs order itself, so this avoids two
    independent scan budgets without changing histogram bounds or correlation.

The runtime evidence is deliberately limited:

  • N=385,C=64 selected B=7 and emitted the expected 56 of 56 table-Region
    requests with zero retries.
  • One N=449,C=64 resource comparison emitted 66 requests instead of 449
    (-85.3%). Its single paired wall-clock delta was +0.65%, which is not a
    performance conclusion. Statistics and checksums were identical; both arms
    stayed under the predefined resource gates and showed no backoff, OOM, swap,
    message-size, or fatal signal.
  • Existing N=257,B=5 twelve-pair evidence was performance-neutral. The
    N=449,B=7 result is one pair only. This PR does not claim universal speedup
    or memory savings.
  • Multi-round B=3 and B=7, more than four stores, and a real-cluster
    N<=2C gate remain unmeasured.

The temporary dependency revisions are validation pins:

  • kvproto: a29e794b680d (current compatible base plus the merged-child
    response contract)
  • client-go: 949253a114a6 (current compatible base plus nested child resource
    accounting)

This Draft cannot land until the corresponding upstream protocol and
client-go changes land and all consumers are repinned to mutually compatible
upstream revisions. These contributor refs are not a dependency exit.

Check List

Tests

  • Unit test
    • ./tools/check/failpoint-go-test.sh pkg/executor -run '^TestAnalyzeBuildsSingleBatchableRequest$' -count=1
    • ./tools/check/failpoint-go-test.sh pkg/store/copr -run '^(TestBuildTasksWithoutBuckets|TestAnalyzeStoreBatchPolicy|TestAnalyzeStoreBatchWidthUsesRequestRegionCount|TestAnalyzeStoreBatchWidthMatchesSmallIntegerReference|TestHandleBatchCopResponseMergedAndUnansweredTasks|TestHandleBatchCopResponseUpdatesChildBucketsOnVersionNotMatch)$' -count=1
    • ./tools/check/failpoint-go-test.sh pkg/store/copr/copr_test -run '^(TestAnalyzeBatchUsesRequestWideDistinctRegionCount|TestBuildCopIteratorWithBatchStoreCopr|TestBuildCopIteratorWithRunawayChecker)$' -count=1
    • make fmt
    • make lint
    • go mod tidy (zero diff)
    • go mod verify
    • make bazel_prepare (two consecutive zero-diff runs)
    • git diff --check 93f713cf551f..HEAD
  • Integration test
  • Manual test
    • A pinned 1-PD/4-TiKV/1-TiDB fixture produced the bounded runtime evidence
      listed above; statistics and checksums matched.
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Reduce the number of TiKV requests used by V2 full-sampling ANALYZE for large
physical tables while preserving the configured Analyze scan-concurrency
budget.

@ti-chi-bot ti-chi-bot Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9910e5b6-3119-4e02-9753-2caecc418320

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ti-chi-bot

ti-chi-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@ti-chi-bot ti-chi-bot Bot added contribution This PR is from a community contributor. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 24, 2026
@pingcap-cla-assistant

pingcap-cla-assistant Bot commented Jul 24, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Murphy seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@0xTars
0xTars force-pushed the copper/analyze-batch-merge-v2 branch from 0f49e5e to d19e055 Compare July 24, 2026 12:31
@0xTars
0xTars force-pushed the copper/analyze-batch-merge-v2 branch 2 times, most recently from e9cc26c to f86b451 Compare July 29, 2026 19:13
@ti-chi-bot ti-chi-bot Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 29, 2026
@0xTars
0xTars force-pushed the copper/analyze-batch-merge-v2 branch 4 times, most recently from 3181cc0 to b57f31a Compare July 29, 2026 20:24
Pin the coordinated kvproto and client-go revisions together so the
behavior commit sees both the explicit merged-task acknowledgment field
and nested child-accounting support.

These contributor revisions are validation-only. Before this draft can
land, a live upstream protocol successor must replace the historical
closed kvproto patch, the canonical TiKV implementation must be chosen,
and all consumers must repin to landed upstream revisions.

Signed-off-by: Murphy <poe1024.liu@gmail.com>
@0xTars
0xTars force-pushed the copper/analyze-batch-merge-v2 branch from b57f31a to dbdf961 Compare July 30, 2026 14:13
@ti-chi-bot

ti-chi-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign cfzjywxk, terry1purcell for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Let full-sampling Analyze requests opt in to store-batched coprocessor
tasks, with the batch width derived from the request's own region count
instead of the DAG store-batch session variable.

Width selection: with C the resolved Analyze scan budget (concurrency)
and N the request-wide deduplicated region count, N <= 2*C keeps the
legacy one-task-per-region shape (no batching); otherwise the total
width is B = min(ceil(N/C), max{b : b*(b+1) <= C}), so a batched width
always leaves room for at least B+1 concurrent outer workers. N counts
distinct region IDs once across all KeyRanges partitions, while packing
runs per partition and never merges tasks across a partition boundary.
Every retry and rebuild path leaves the budget zero and thus rebuilds
plain singleton tasks. Analyze ignores tidb_store_batch_size entirely;
the generic DAG store-batch path and the TiKV protocol are unchanged
apart from the explicit merged-task acknowledgment capability flag.

Also fix two numeric issues surfaced while validating the shapes:

  * handleBatchCopResponse counted every remaining retry task as a
    fallback, so a region split during retry could underflow the
    store-batched counter; count the original fallback exactly once.

  * an unsigned-PK table split its Analyze scan into two requests at
    the int64 boundary, making one table use two independent scan
    budgets and region-count decisions. V2 full-sampling Analyze
    derives handle order from samples, so keep all ranges in one
    request under one budget.

Signed-off-by: Murphy <poe1024.liu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution This PR is from a community contributor. do-not-merge/needs-triage-completed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant