Skip to content

planner: arm engine-restricted alternative logical plan rounds from round 1's engine usage (WIP)#70020

Draft
terry1purcell wants to merge 2 commits into
pingcap:masterfrom
terry1purcell:enginerounds2
Draft

planner: arm engine-restricted alternative logical plan rounds from round 1's engine usage (WIP)#70020
terry1purcell wants to merge 2 commits into
pingcap:masterfrom
terry1purcell:enginerounds2

Conversation

@terry1purcell

@terry1purcell terry1purcell commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

This is an alternative design to #70005; only one of the two will ship.

The physical search picks a storage engine per DataSource through local cost comparisons, so per-table index access can win table by table even when a homogeneous whole-statement plan (in particular an MPP plan) would win overall — and that homogeneous alternative is never costed as a unit. #70005 arms the tikv-only / tiflash-only rebuild rounds only when round 1's chosen plan already mixes both engines. This PR instead arms each round from the engines round 1 actually read, so an all-TiKV plan still gets a whole-statement TiFlash/MPP alternative costed against it — the case #70005 structurally cannot reach.

InspectPlanShape walks round 1's chosen plan once for its engine usage and for whether it joins or aggregates, descending into reader subtrees because pushed-down aggregations and MPP joins live inside cop and MPP tasks. The tikv-only round arms when round 1 read from TiFlash anywhere; the tiflash-only round arms when it read from TiKV anywhere, including the all-TiKV case. Shared gates keep the extra rounds off statements that cannot benefit: a join or aggregation must be present (keeps the rounds off the OLTP fast path), no READ_FROM_STORAGE hint may pin an engine, and enforced MPP is excluded (its cost discount would distort the cross-round comparison). The tiflash-only round additionally requires every table to have a TiFlash access path and MPP to be allowed.

What problem does this PR solve?

Issue Number: ref #70021

Problem Summary:

The alternative logical plan driver only builds a fully engine-homogeneous plan when round 1 already mixed engines. An all-TiKV plan can still hide a cheaper whole-statement MPP plan that was never costed, and that case is not covered.

What changed and how does it work?

  • New physicalop.InspectPlanShape returns, from a single walk of round 1's chosen plan, which storage engines it read from and whether it contains a join or aggregation. It descends into reader subtrees (TablePlan / IndexPlan / partial plans) because pushed-down aggregations and MPP joins live inside cop and MPP tasks.
  • optimize records those facts as StmtCtx signals after round 1 and pre-computes round eligibility from them (so later rounds' side-effect signals cannot leak into arming).
  • Two rounds, tikv-only and tiflash-only, rebuild the plan with IsolationReadEngines restricted to a single storage engine; the existing strict-< cross-round cost comparison picks the winner. Arming and gates are described above.
  • Per-round setup now returns its own cleanup closure instead of using package-level saved state, so concurrent optimize calls in different sessions cannot clobber one another's saved values.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

TestAlternativeEngineRoundArming in pkg/planner/core/casetest/mpp/engine_rounds_test.go covers each arming and gating case, including the headline all-TiKV-plan-arms-tiflash-only case, the missing-replica skip, the join/agg gate, the hint gate, enforced MPP, and the feature-off case.

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.

None

🤖 Generated with Claude Code

https://claude.ai/code/session_01TvP1rNMDfN8B6S7jx14Y7M

Adds tikv-only and tiflash-only alternative logical plan rounds that arm
from the storage engines round 1's chosen plan actually read, rather than
requiring that plan to mix engines. An all-TiKV plan can still hide a
better whole-statement MPP plan: the bottom-up search picks an engine per
DataSource through local cost comparisons, so per-table index access can
win table by table even when a homogeneous TiFlash plan would win overall,
and that alternative is otherwise never costed.

InspectPlanShape walks round 1's plan for engine usage and join/agg
presence, descending into reader subtrees because pushed-down aggregations
and MPP joins live inside cop and MPP tasks.

Gates keep the extra rounds off statements that cannot benefit: a join or
aggregation must be present, no READ_FROM_STORAGE hint may pin an engine,
enforced MPP is excluded (its cost discount would distort the cross-round
comparison), and tiflash-only additionally requires every table to have a
TiFlash access path and MPP to be allowed.

This is an alternative design to pingcap#70005; only one of the two will ship.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvP1rNMDfN8B6S7jx14Y7M
@ti-chi-bot

ti-chi-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. 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: 1bb67e8b-5246-4a17-a99f-f524dc16e7fc

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 added sig/planner SIG: Planner size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 24, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 24, 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 elsa0520 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

BenchmarkEngineRoundOptimize measures per-statement optimization time for
the alternative logical plan driver on the query class where this design
and the mixed-engine-only design (pingcap#70005) diverge: an all-TiKV join that
this branch arms the tiflash-only round for. A scan-only case confirms the
join/agg gate leaves the OLTP fast path untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvP1rNMDfN8B6S7jx14Y7M
@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 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner 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