Skip to content

ddl: enhance placement rule mismatch error#69722

Open
Meowooh wants to merge 1 commit into
pingcap:masterfrom
Meowooh:issue-47253-placement-rule-error
Open

ddl: enhance placement rule mismatch error#69722
Meowooh wants to merge 1 commit into
pingcap:masterfrom
Meowooh:issue-47253-placement-rule-error

Conversation

@Meowooh

@Meowooh Meowooh commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #47253

Problem Summary:

When ALTER TABLE ... PLACEMENT POLICY ... fails because PD rejects a generated placement rule, TiDB only returns PD's raw error, for example can not match any store. The message does not tell users which table, placement policy, rule group, rule, or constraints caused the mismatch, making label/topology issues hard to diagnose.

What changed and how does it work?

  • Add a DDL-layer helper to enhance PD placement rule HTTP errors while preserving infosync.ErrHTTPServiceError and MySQL error code 8243.
  • In onAlterTablePlacement, when infosync.PutRuleBundlesWithDefaultRetry fails, parse the PD error's rule group/rule ID and look up the matching rule in the current TiDB placement bundle.
  • Append TiDB context to the original PD error message, including table, policy, rule group, rule, role, count, and label constraints.
  • Add the No store matches... SHOW PLACEMENT LABELS hint only for the can not match any store case.
  • Trim leading/trailing whitespace from the PD body before appending context so the enhanced message remains readable.

Example enhanced message:

before

ERROR 8243 (HY000): "[PD:placement:ErrRuleContent]invalid rule content, rule 'table_rule_105_1' from rule group 'TiDB_DDL_105' can not match any store"

now

ERROR 8243 (HY000): "[PD:placement:ErrRuleContent]invalid rule content, rule 'table_rule_120_3' from rule group 'TiDB_DDL_120' can not match any store"; TiDB placement context: table=`test47253_e2e`.`t`, policy=`mydeploy`, rule_group=`TiDB_DDL_120`, rule=`table_rule_120_3`, role=voter, count=1, label_constraints=[region in (us-west)]. No store matches the failed rule constraints; check available store labels with SHOW PLACEMENT LABELS.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Unit test:

go test ./pkg/ddl -run TestEnhancePlacementRuleError -count=1

Manual test:

  • Started a temporary PD/TiKV topology with store labels matching the issue scenario:
    • 2 stores with region=us-east-1
    • 2 stores with region=us-east-2
    • 1 store with region=us-west-1
  • Created all_in_east and mydeploy placement policies, where mydeploy requests missing +region=us-west.
  • Verified:
    • ALTER TABLE t PLACEMENT POLICY mydeploy still returns code 8243.
    • The error message includes table, policy, rule group, rule, role/count, and label_constraints=[region in (us-west)].
    • SHOW WARNINGS contains the same enhanced message.
    • Failed DDL leaves the table on the previous placement policy.
    • A normal placement policy path still succeeds.

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

Improve the error message for placement rule store-label mismatches by including the affected table, placement policy, failed rule, and label constraints.

Summary by CodeRabbit

  • New Features

    • Improved placement-related error messages with clearer context, including schema/table names, policy details, rule information, and label constraints.
  • Bug Fixes

    • Added a helpful hint for placement failures when no store matches the required rules, including guidance to check placement labels.
    • Preserved existing error behavior for unrelated errors while making placement update failures easier to diagnose.

@ti-chi-bot ti-chi-bot Bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 8, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

Hi @Meowooh. 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 8, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 8, 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 d3hunter 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

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new pkg/ddl/placement_error.go module that enriches PD placement rule HTTP errors with TiDB-specific context (schema/table/policy, rule role/count/label constraints), wires it into onAlterTablePlacement in pkg/ddl/table.go, and adds corresponding unit tests.

Changes

Placement error context enhancement

Layer / File(s) Summary
Placement error formatting module
pkg/ddl/placement_error.go
Adds placementRuleErrorWithContext and helpers to extract rule/group IDs, locate rules in bundles, and format table names, identifiers, and label constraints into an enriched "TiDB placement context" message, including a "SHOW PLACEMENT LABELS" hint when applicable.
Wire enhancement into alter table placement flow
pkg/ddl/table.go
onAlterTablePlacement builds a bundles slice, calls PutRuleBundlesWithDefaultRetry conditionally on non-nil bundle, and on failure cancels the job returning an error wrapped via placementRuleErrorWithContext with schema/table/policy context.
Unit tests for error enhancement
pkg/ddl/placement_error_test.go
Adds tests covering enhanced error message content, omission of the no-store hint, rule-not-found fallback messaging, and pass-through behavior for non-PD errors.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant onAlterTablePlacement
  participant PDClient
  participant placementRuleErrorWithContext
  participant findPlacementRule

  onAlterTablePlacement->>PDClient: PutRuleBundlesWithDefaultRetry(bundles)
  PDClient-->>onAlterTablePlacement: ErrHTTPServiceError
  onAlterTablePlacement->>placementRuleErrorWithContext: wrap error with schema/table/policy/bundles
  placementRuleErrorWithContext->>placementRuleErrorWithContext: extractPlacementRuleError(message)
  placementRuleErrorWithContext->>findPlacementRule: locate rule by id in bundles
  findPlacementRule-->>placementRuleErrorWithContext: rule or not found
  placementRuleErrorWithContext-->>onAlterTablePlacement: enhanced error message
Loading

Poem

A rule went missing, stores said "no way,"
Now TiDB whispers just where things went astray.
With schema and policy tucked in the text,
No more guessing what error comes next! 🐰
Hop along, dear rules, you're context-blessed now.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR adds the requested placement-error diagnostics for #47253, including table, policy, rule group, rule, and constraint details.
Out of Scope Changes check ✅ Passed The code changes stay focused on placement error enrichment and tests, with no obvious unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely summarizes the main change: improving placement rule mismatch errors.
Description check ✅ Passed The description follows the template well, including issue reference, problem summary, change details, tests, side effects, and release note.
✨ 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.

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. 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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve the error report of placement rule

1 participant