Skip to content

tables: force to lock the touched index in DML when DDL merging temp index#62387

Merged
ti-chi-bot[bot] merged 2 commits into
pingcap:masterfrom
lcwangchao:force_lock_index_when_merge_tempidx
Jul 14, 2025
Merged

tables: force to lock the touched index in DML when DDL merging temp index#62387
ti-chi-bot[bot] merged 2 commits into
pingcap:masterfrom
lcwangchao:force_lock_index_when_merge_tempidx

Conversation

@lcwangchao
Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #62337

Problem Summary:

see descriptions in issue. We choose the second way to fix it.

What changed and how does it work?

When the index is in state BackfillStateReadyToMerge or BackfillStateMerging, we also lock the touched non-unique indices to ensure the correctness, even if the DDL does not lock the row.

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.

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

@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 14, 2025
@lcwangchao lcwangchao force-pushed the force_lock_index_when_merge_tempidx branch from 4761250 to 444606e Compare July 14, 2025 04:03
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.4431%. Comparing base (c765578) to head (6142d72).
Report is 7 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #62387        +/-   ##
================================================
+ Coverage   72.9000%   73.4431%   +0.5431%     
================================================
  Files          1754       1755         +1     
  Lines        484967     488074      +3107     
================================================
+ Hits         353541     358457      +4916     
+ Misses       109800     107982      -1818     
- Partials      21626      21635         +9     
Flag Coverage Δ
integration 42.7042% <51.5151%> (?)
unit 72.1538% <100.0000%> (-0.0067%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 52.7804% <ø> (ø)
parser ∅ <ø> (∅)
br 46.3170% <ø> (+0.0076%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread pkg/table/tables/index.go
// to make sure the serialization between the DDL and DML transactions.
func (c *index) mayDDLMergingTempIndex() bool {
return c.idxInfo.BackfillState == model.BackfillStateReadyToMerge ||
c.idxInfo.BackfillState == model.BackfillStateMerging
Copy link
Copy Markdown
Collaborator Author

@lcwangchao lcwangchao Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tangenta Please to review this method. It means when ddl worker enters the state BackfillStateReadyToMerge , all the DML should have seen the BackfillStateReadyToMerge or the previous state

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lcwangchao Better to add comments about the constraint ensured by DDL or the MDL mechanism

@lcwangchao
Copy link
Copy Markdown
Collaborator Author

/retest

@ti-chi-bot ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 14, 2025
Comment thread pkg/session/test/tidb_test.go Outdated
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jul 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, tangenta

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

The pull request process is described 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

@ti-chi-bot ti-chi-bot Bot added the lgtm label Jul 14, 2025
@lcwangchao lcwangchao requested a review from Benjamin2037 July 14, 2025 07:45
@ti-chi-bot ti-chi-bot Bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jul 14, 2025
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jul 14, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-07-14 07:36:52.297599184 +0000 UTC m=+2503665.020778166: ☑️ agreed by tangenta.
  • 2025-07-14 07:45:11.81643049 +0000 UTC m=+2504164.539609494: ☑️ agreed by D3Hunter.

Co-authored-by: D3Hunter <jujj603@gmail.com>
@lcwangchao lcwangchao added needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. require-LGT3 Indicates that the PR requires three LGTM. labels Jul 14, 2025
@Benjamin2037 Benjamin2037 added needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. labels Jul 14, 2025
@ti-chi-bot ti-chi-bot Bot merged commit 95b5aa9 into pingcap:master Jul 14, 2025
24 of 27 checks passed
@ti-chi-bot
Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-7.5: #62397.
But this PR has conflicts, please resolve them!

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 14, 2025
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 14, 2025
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-8.1: #62398.
But this PR has conflicts, please resolve them!

@ti-chi-bot
Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #62400.
But this PR has conflicts, please resolve them!

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 14, 2025
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@lcwangchao lcwangchao deleted the force_lock_index_when_merge_tempidx branch July 14, 2025 09:32
ti-chi-bot Bot pushed a commit that referenced this pull request Jul 15, 2025
ti-chi-bot Bot pushed a commit that referenced this pull request Jul 17, 2025
wjhuang2016 added a commit to wjhuang2016/tidb that referenced this pull request Jul 18, 2025
@wjhuang2016 wjhuang2016 mentioned this pull request Jul 18, 2025
13 tasks
ti-chi-bot Bot pushed a commit that referenced this pull request Jul 21, 2025
tangenta added a commit to tangenta/tidb that referenced this pull request Jul 24, 2025
tangenta added a commit to tangenta/tidb that referenced this pull request Jul 28, 2025
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 30, 2025
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note-none Denotes a PR that doesn't merit a release note. require-LGT3 Indicates that the PR requires three LGTM. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

txn: support secondary index key write conflict check for pessimistic transactions considering DDL fast add index

6 participants