*: Allow Point_Get during DDL with Global Index #56382

mjonss · 2024-09-27T11:46:40Z

What problem does this PR solve?

Issue Number: ref #55819 , ref #45133 , close #56819

Problem Summary:
During DDL that affects Global Index, the PointGet access method is blocked, which may cause temporary bad query plans during partitioning management commands, like DROP/TRUNCATE/REORGANIZE PARTITION.

What changed and how does it work?

Allow PointGet, by filter away the partitions that should not be seen (like when the cleanup of the global index is ongoing).

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

Release note

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

None

tiprow · 2024-09-27T11:46:59Z

Hi @mjonss. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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.

codecov · 2024-09-27T11:59:09Z

Codecov Report

Attention: Patch coverage is 93.91304% with 7 lines in your changes missing coverage. Please review.

Project coverage is 57.4851%. Comparing base (7ce5bd7) to head (fdbae50).
Report is 4 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #56382         +/-   ##
=================================================
- Coverage   73.3565%   57.4851%   -15.8714%     
=================================================
  Files          1636       1785        +149     
  Lines        453398     652320     +198922     
=================================================
+ Hits         332597     374987      +42390     
- Misses       100440     252093     +151653     
- Partials      20361      25240       +4879

Flag	Coverage Δ
integration	`39.5727% <62.1621%> (?)`
unit	`72.4745% <74.7747%> (-0.0967%)`	⬇️

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

Components	Coverage Δ
dumpling	`52.9478% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`62.4134% <ø> (+16.3807%)`	⬆️

mjonss · 2024-09-27T15:15:20Z

pkg/meta/model/table.go

+// session should not see (may be duplicate errors on insert/update though)
+// For example during truncate or drop partition.
+func (pi *PartitionInfo) IDsInDDLToIgnore() []int64 {
+	// TODO:


There are multiple PRs/enhancements that will build upon this, especially for DROP and TRUNCATE partition.

mjonss · 2024-09-28T09:03:47Z

/retest

tiprow · 2024-09-28T09:04:09Z

@mjonss: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

mjonss · 2024-09-28T15:50:13Z

Splitted this part from #55819 since it can be reviewed separately.

…-get-during-ddl

pkg/ddl/partition.go

pkg/planner/core/find_best_task.go

…-get-during-ddl

pkg/executor/point_get.go

…-get-during-ddl

pkg/ddl/partition.go

pkg/meta/model/table.go

mjonss · 2024-10-25T11:06:55Z

/retest

tiprow · 2024-10-25T11:07:17Z

@mjonss: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

pkg/planner/core/find_best_task.go

ti-chi-bot · 2024-10-25T11:19:18Z

[LGTM Timeline notifier]

Timeline:

2024-10-25 10:53:56.700718929 +0000 UTC m=+1549.539874470: ☑️ agreed by Defined2014.
2024-10-25 11:19:17.869087724 +0000 UTC m=+3070.708243270: ☑️ agreed by winoros.

mjonss · 2024-10-25T12:17:28Z

/retest

mjonss · 2024-10-25T12:26:11Z

/retest

mjonss · 2024-10-25T12:40:00Z

/retest

ti-chi-bot · 2024-10-25T14:58:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Defined2014, tangenta, winoros

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Defined2014,tangenta,winoros]
~~pkg/ddl/OWNERS~~ [tangenta]
~~pkg/meta/OWNERS~~ [tangenta]
~~pkg/planner/OWNERS~~ [winoros]

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

mjonss · 2024-10-25T15:52:15Z

/retest

ti-chi-bot · 2024-10-25T16:21:28Z

In response to a cherrypick label: new pull request created to branch release-8.4: #56849.

ref pingcap#45133, ref pingcap#55819, close pingcap#56819

ref #45133, ref #55819, close #56819

*: Allow Point_Get during DDL with Global Index

82ed64a

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. sig/planner SIG: Planner labels Sep 27, 2024

mjonss added 5 commits September 27, 2024 14:37

Added comment

ace4c13

Added back check for empty set of partitions after pruning

25d56a7

reshuffled code

e9424d8

Added todo comment

c4f0a13

Clarified comment

3a1271a

mjonss commented Sep 27, 2024

View reviewed changes

mjonss requested review from Defined2014 and qw4990 September 28, 2024 15:50

Merge remote-tracking branch 'pingcap/master' into global-index-point…

20aabb5

…-get-during-ddl

Defined2014 reviewed Sep 29, 2024

View reviewed changes

pkg/ddl/partition.go Outdated Show resolved Hide resolved

pkg/planner/core/find_best_task.go Show resolved Hide resolved

pkg/planner/core/find_best_task.go Show resolved Hide resolved

pkg/planner/core/find_best_task.go Show resolved Hide resolved

Merge remote-tracking branch 'pingcap/master' into global-index-point…

b7735b4

…-get-during-ddl

qw4990 reviewed Sep 30, 2024

View reviewed changes

pkg/executor/point_get.go Show resolved Hide resolved

mjonss added 3 commits October 4, 2024 13:10

Added intest assertion

5d0903e

Merge remote-tracking branch 'pingcap/master' into global-index-point…

3974c7e

…-get-during-ddl

Linting

d4b94fc

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2024

mjonss added 3 commits October 22, 2024 13:45

Merge remote-tracking branch 'pingcap/master' into global-index-point…

ba60603

…-get-during-ddl

Manual merge fixes

c073881

Removed duplicated line

0883f7c

mjonss mentioned this pull request Oct 24, 2024

Row not found for Global Index during DeleteOnly state of Reorganize Partition #56819

Closed

ti-chi-bot bot added the do-not-merge/needs-triage-completed label Oct 25, 2024

Defined2014 reviewed Oct 25, 2024

View reviewed changes

pkg/ddl/partition.go Show resolved Hide resolved

Defined2014 reviewed Oct 25, 2024

View reviewed changes

pkg/meta/model/table.go Outdated Show resolved Hide resolved

Removed duplicated lines.

731d794

ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2024

Defined2014 approved these changes Oct 25, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 25, 2024

winoros reviewed Oct 25, 2024

View reviewed changes

pkg/planner/core/find_best_task.go Outdated Show resolved Hide resolved

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Oct 25, 2024

winoros approved these changes Oct 25, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Oct 25, 2024

mjonss added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 25, 2024

using intest.Assert instead of intest.InTest.

fdbae50

mjonss removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 25, 2024

tangenta approved these changes Oct 25, 2024

View reviewed changes

ti-chi-bot bot added approved needs-cherry-pick-release-8.4 Should cherry pick this PR to release-8.4 branch. labels Oct 25, 2024

ti-chi-bot bot merged commit 33c30ee into pingcap:master Oct 25, 2024
26 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Oct 25, 2024

*: Allow Point_Get during DDL with Global Index (pingcap#56382)

7e6b49b

ref pingcap#45133, ref pingcap#55819, close pingcap#56819

ti-chi-bot mentioned this pull request Oct 25, 2024

*: Allow Point_Get during DDL with Global Index (#56382) #56849

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Oct 27, 2024

*: Allow Point_Get during DDL with Global Index (#56382) (#56849)

2511e92

ref #45133, ref #55819, close #56819

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: Allow Point_Get during DDL with Global Index #56382

*: Allow Point_Get during DDL with Global Index #56382

mjonss commented Sep 27, 2024 •

edited

Loading

tiprow bot commented Sep 27, 2024

codecov bot commented Sep 27, 2024 •

edited

Loading

mjonss Sep 27, 2024

mjonss commented Sep 28, 2024

tiprow bot commented Sep 28, 2024

mjonss commented Sep 28, 2024

mjonss commented Oct 25, 2024

tiprow bot commented Oct 25, 2024

ti-chi-bot bot commented Oct 25, 2024

mjonss commented Oct 25, 2024

mjonss commented Oct 25, 2024

mjonss commented Oct 25, 2024

ti-chi-bot bot commented Oct 25, 2024

mjonss commented Oct 25, 2024

ti-chi-bot commented Oct 25, 2024

*: Allow Point_Get during DDL with Global Index #56382

*: Allow Point_Get during DDL with Global Index #56382

Conversation

mjonss commented Sep 27, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Sep 27, 2024

codecov bot commented Sep 27, 2024 • edited Loading

Codecov Report

mjonss Sep 27, 2024

Choose a reason for hiding this comment

mjonss commented Sep 28, 2024

tiprow bot commented Sep 28, 2024

mjonss commented Sep 28, 2024

mjonss commented Oct 25, 2024

tiprow bot commented Oct 25, 2024

ti-chi-bot bot commented Oct 25, 2024

[LGTM Timeline notifier]

mjonss commented Oct 25, 2024

mjonss commented Oct 25, 2024

mjonss commented Oct 25, 2024

ti-chi-bot bot commented Oct 25, 2024

mjonss commented Oct 25, 2024

ti-chi-bot commented Oct 25, 2024

mjonss commented Sep 27, 2024 •

edited

Loading

codecov bot commented Sep 27, 2024 •

edited

Loading