ddl: skip adding index reorg work for empty tables #56406

tangenta · 2024-09-29T08:14:40Z

What problem does this PR solve?

Issue Number: close #54230, close #51421

Problem Summary:

We can reduce the overhead of data reorganization or distributed framework scheduling if the target table is empty.

What changed and how does it work?

See code changes.

Check List

Tests

Unit test
Integration test

Manual test (add detailed scripts or steps below)
Before this PR (empty tables):

mysql> create index idx_listid on tt1(id,listid);
Query OK, 0 rows affected (2.52 sec)

mysql> create index idx_listid on tt2(listid);
Query OK, 0 rows affected (1.52 sec)

After this PR (empty tables):

mysql> create index idx_listid on tt1(id,listid);
Query OK, 0 rows affected (0.34 sec)

mysql> create index idx_listid on tt2(listid);
Query OK, 0 rows affected (0.29 sec)

Before this PR (non-empty tables):

mysql> create index idx_listid on tt1(id,listid);
Query OK, 0 rows affected (2.49 sec)

mysql> create index idx_listid on tt2(listid);
Query OK, 0 rows affected (2.16 sec)

After this PR (non-empty tables):

mysql> create index idx_listid on tt1(id,listid);
Query OK, 0 rows affected (2.57 sec)

mysql> create index idx_listid on tt2(listid);
Query OK, 0 rows affected (2.20 sec)

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-29T08:14:58Z

Hi @tangenta. 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.

pkg/session/bootstrap.go

codecov · 2024-09-29T08:33:21Z

Codecov Report

Attention: Patch coverage is 90.85366% with 15 lines in your changes missing coverage. Please review.

Project coverage is 57.3725%. Comparing base (ed9a909) to head (da86bd9).
Report is 8 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #56406         +/-   ##
=================================================
- Coverage   73.2825%   57.3725%   -15.9100%     
=================================================
  Files          1636       1786        +150     
  Lines        453340     643316     +189976     
=================================================
+ Hits         332219     369087      +36868     
- Misses       100768     248976     +148208     
- Partials      20353      25253       +4900

Flag	Coverage Δ
integration	`39.5413% <85.9756%> (?)`
unit	`72.6393% <90.8536%> (+0.1537%)`	⬆️

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

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

D3Hunter

lgtm except existing comments

ti-chi-bot · 2024-09-29T08:48:55Z

[LGTM Timeline notifier]

Timeline:

2024-09-29 08:27:28.756711325 +0000 UTC m=+170604.176924336: ☑️ agreed by lance6716.
2024-09-29 08:48:54.421181701 +0000 UTC m=+171889.841394714: ☑️ agreed by D3Hunter.

Benjamin2037

rest LGTM

D3Hunter · 2024-09-29T08:57:48Z

/hold

please pass ctx in bootstrapSessionImpl to rebuildAllPartitionValueMapAndSorted. you can unhold youself after that

tangenta · 2024-09-29T09:30:52Z

/hold

please pass ctx in bootstrapSessionImpl to rebuildAllPartitionValueMapAndSorted. you can unhold youself after that

Done.

yudongusa

LGTM

tangenta · 2024-10-01T15:10:09Z

/retest

tiprow · 2024-10-01T15:10:32Z

@tangenta: 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.

tangenta · 2024-10-08T02:59:35Z

/ok-to-test

tangenta · 2024-10-08T02:59:39Z

/retest

tangenta · 2024-10-09T02:22:59Z

/retest

tangenta · 2024-10-09T02:35:37Z

/retest

go: downloading github.com/pingcap/failpoint v0.0.0-20240527053858-9b3b6e34194a
go: downloading github.com/sergi/go-diff v1.1.0
bazel --output_user_root=/home/jenkins/.tidb/tmp run --config=ci --repository_cache=/share/.cache/bazel-repository-cache //:gazelle
make: execvp: bazel: Permission denied
make: *** [Makefile:591: bazel_ci_simple_prepare] Error 127
./build/jenkins_unit_test.sh: line 26: bazel_collect: command not found
mv: cannot stat 'bazel.xml': No such file or directory
script returned exit code 2

ti-chi-bot · 2024-10-16T03:58:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, D3Hunter, lance6716, yudongusa

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~~ [Benjamin2037,D3Hunter,lance6716]
~~pkg/ddl/OWNERS~~ [Benjamin2037,D3Hunter,lance6716]

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

tangenta · 2024-10-22T01:09:50Z

/unhold

tangenta · 2024-10-22T06:40:56Z

/retest

    plan_test.go:40: 
        	Error Trace:	pkg/planner/core/casetest/plan_test.go:40
        	            				pkg/planner/core/casetest/tiflash_selection_late_materialization_test.go:76
        	Error:      	Not equal: 
        	            	expected: 4
        	            	actual  : 3
        	Test:       	TestTiFlashLateMaterialization


    plan_stats_test.go:81: 
        	Error Trace:	pkg/planner/core/casetest/planstats/plan_stats_test.go:81
        	            				pkg/planner/core/casetest/planstats/plan_stats_test.go:210
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : -1
        	Test:       	TestPlanStatsLoad

tangenta · 2024-10-22T07:59:23Z

/retest

    testkit.go:161: 
        	Error Trace:	pkg/testkit/testkit.go:161
        	            				pkg/testkit/testkit.go:154
        	            				pkg/planner/core/casetest/instanceplancache/concurrency_test.go:84
        	            				src/runtime/asm_amd64.s:1700
        	Error:      	Received unexpected error:
        	            	[executor:1213]Deadlock found when trying to get lock; try restarting transaction
        	            	github.com/pingcap/errors.AddStack
        	            		external/com_github_pingcap_errors/errors.go:178
        	            	github.com/pingcap/errors.Trace
        	            		external/com_github_pingcap_errors/juju_adaptor.go:15
        	            	github.com/pingcap/tidb/pkg/testkit.(*TestKit).ExecWithContext
        	            		pkg/testkit/testkit.go:443
        	            	github.com/pingcap/tidb/pkg/testkit.(*TestKit).MustExecWithContext
        	            		pkg/testkit/testkit.go:159
        	            	github.com/pingcap/tidb/pkg/testkit.(*TestKit).MustExec
        	            		pkg/testkit/testkit.go:154
        	            	pkg/planner/core/casetest/instanceplancache/instanceplancache_test.(*worker).run
        	            		pkg/planner/core/casetest/instanceplancache/concurrency_test.go:84
        	            	runtime.goexit
        	            		src/runtime/asm_amd64.s:1700
        	Test:       	TestInstancePlanCacheConcurrencySysbench
        	Messages:   	sql:update normal.sbtest set c='4639' where id=157, [], error stack [executor:1213]Deadlock found when trying to get lock; try restarting transaction
        	            	github.com/pingcap/errors.AddStack
        	            		external/com_github_pingcap_errors/errors.go:178
        	            	github.com/pingcap/errors.Trace
        	            		external/com_github_pingcap_errors/juju_adaptor.go:15
        	            	github.com/pingcap/tidb/pkg/testkit.(*TestKit).ExecWithContext
        	            		pkg/testkit/testkit.go:443
        	            	github.com/pingcap/tidb/pkg/testkit.(*TestKit).MustExecWithContext
        	            		pkg/testkit/testkit.go:159
        	            	github.com/pingcap/tidb/pkg/testkit.(*TestKit).MustExec
        	            		pkg/testkit/testkit.go:154
        	            	pkg/planner/core/casetest/instanceplancache/instanceplancache_test.(*worker).run
        	            		pkg/planner/core/casetest/instanceplancache/concurrency_test.go:84
        	            	runtime.goexit
        	            		src/runtime/asm_amd64.s:1700

tangenta · 2024-10-23T02:51:02Z

/retest

    queue_v2_ddl_handler_test.go:437: 
        	Error Trace:	pkg/statistics/handle/autoanalyze/priorityqueue/queue_v2_ddl_handler_test.go:437
        	Error:      	Not equal: 
        	            	expected: 114
        	            	actual  : 110
        	Test:       	TestExchangeTablePartition

https://do.pingcap.net/jenkins/blue/rest/organizations/jenkins/pipelines/pingcap/pipelines/tidb/pipelines/pull_unit_test_ddlv1/runs/256/nodes/63/steps/69/log/?start=0

    distsql_test.go:346: 
        	Error Trace:	pkg/executor/distsql_test.go:346
        	Error:      	"1" is not greater than "2"
        	Test:       	TestCoprocessorPagingSize

https://do.pingcap.net/jenkins/blue/rest/organizations/jenkins/pipelines/pingcap/pipelines/tidb/pipelines/ghpr_unit_test/runs/20924/nodes/63/steps/69/log/?start=0

tangenta · 2024-10-23T07:10:43Z

/retest

#56797

    split_table_test.go:64: 
        	Error Trace:	pkg/executor/test/splittest/split_table_test.go:64
        	Error:      	Expect "t_112_5f7203800000000000000183800000000000" to match "t_115_"
        	Test:       	TestClusterIndexShowTableRegion

https://do.pingcap.net/jenkins/blue/rest/organizations/jenkins/pipelines/pingcap/pipelines/tidb/pipelines/ghpr_unit_test/runs/20982/nodes/63/steps/69/log/?start=0

tangenta added 3 commits September 29, 2024 15:59

ddl: skip adding index reorg work for empty tables

ec282c9

refine test code

eac776f

Merge remote-tracking branch 'upstream/master' into add-index-skip-reorg

8ed23d3

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 Sep 29, 2024

fix linter

60b1a2d

Benjamin2037 reviewed Sep 29, 2024

View reviewed changes

pkg/session/bootstrap.go Outdated Show resolved Hide resolved

lance6716 approved these changes Sep 29, 2024

View reviewed changes

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

D3Hunter approved these changes Sep 29, 2024

View reviewed changes

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

Benjamin2037 approved these changes Sep 29, 2024

View reviewed changes

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 29, 2024

address comment and fix TestAddIndexMultipleDelete

83b1490

tangenta added 7 commits September 29, 2024 17:48

fix TestAddIndexMergeVersionIndexValue

04b322d

refine function name

4836b7d

move empty check into doReorgWorkForCreateIndex

a92a3e7

refine naming

71cce20

fix code and linter

084c68f

consistent naming

b416be3

return if job.SnapshotVersion is not zero

4250966

yudongusa approved these changes Sep 29, 2024

View reviewed changes

ti-chi-bot bot added the approved label Sep 29, 2024

fix linter

afc6035

send asc scan request instead

7dbedd1

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 30, 2024

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

dont rely on stat cache

9fe9991

fix TestDumpPlanReplayerAPI

477b41b

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

Merge remote-tracking branch 'upstream/master' into HEAD

0c08e25

fix TestDumpPlanReplayerAPI

63ec9cb

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

tangenta added 3 commits October 17, 2024 18:02

test: fix TestAddGlobalIndexInIngest

f3a9278

Merge remote-tracking branch 'upstream/master' into HEAD

519180b

Merge remote-tracking branch 'upstream/master' into HEAD

0b6423c

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 22, 2024

tangenta added 3 commits October 24, 2024 09:11

Merge remote-tracking branch 'upstream/master' into HEAD

a4c44c4

set label for topsql

4a5e30a

fix TestPauseFailedOnCommit

da86bd9

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: skip adding index reorg work for empty tables #56406

ddl: skip adding index reorg work for empty tables #56406

tangenta commented Sep 29, 2024 •

edited

Loading

tiprow bot commented Sep 29, 2024

codecov bot commented Sep 29, 2024 •

edited

Loading

D3Hunter left a comment

ti-chi-bot bot commented Sep 29, 2024

Benjamin2037 left a comment

D3Hunter commented Sep 29, 2024

tangenta commented Sep 29, 2024

yudongusa left a comment

tangenta commented Oct 1, 2024

tiprow bot commented Oct 1, 2024

tangenta commented Oct 8, 2024

tangenta commented Oct 8, 2024

tangenta commented Oct 9, 2024

tangenta commented Oct 9, 2024

ti-chi-bot bot commented Oct 16, 2024

tangenta commented Oct 22, 2024

tangenta commented Oct 22, 2024

tangenta commented Oct 22, 2024

tangenta commented Oct 23, 2024

tangenta commented Oct 23, 2024

ddl: skip adding index reorg work for empty tables #56406

ddl: skip adding index reorg work for empty tables #56406

Conversation

tangenta commented Sep 29, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Sep 29, 2024

codecov bot commented Sep 29, 2024 • edited Loading

Codecov Report

D3Hunter left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 29, 2024

[LGTM Timeline notifier]

Benjamin2037 left a comment

Choose a reason for hiding this comment

D3Hunter commented Sep 29, 2024

tangenta commented Sep 29, 2024

yudongusa left a comment

Choose a reason for hiding this comment

tangenta commented Oct 1, 2024

tiprow bot commented Oct 1, 2024

tangenta commented Oct 8, 2024

tangenta commented Oct 8, 2024

tangenta commented Oct 9, 2024

tangenta commented Oct 9, 2024

ti-chi-bot bot commented Oct 16, 2024

tangenta commented Oct 22, 2024

tangenta commented Oct 22, 2024

tangenta commented Oct 22, 2024

tangenta commented Oct 23, 2024

tangenta commented Oct 23, 2024

tangenta commented Sep 29, 2024 •

edited

Loading

codecov bot commented Sep 29, 2024 •

edited

Loading