br: reused table id is disabled when restore a brand-new cluster by fengou1 · Pull Request #41358 · pingcap/tidb

fengou1 · 2023-02-13T14:03:57Z

What problem does this PR solve?

Issue Number: close #41333

Problem Summary:
when restoring to a brand-new cluster, reused table id was disabled, which caused restore to consume more resources.

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

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

ti-chi-bot · 2023-02-13T14:04:00Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

3pointer
YuJuncen

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

YuJuncen

rest lgtm

br/pkg/gluetidb/glue_test.go

Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>

3pointer · 2023-02-14T02:10:49Z

br/pkg/gluetidb/glue.go

 		}
 		mid := len(info) / 2
-		err = gs.SplitBatchCreateTable(schema, info[:mid])
+		err = gs.SplitBatchCreateTable(schema, info[:mid], cs...)


Maybe do not use optional arguments here?

remote opt parameter in dd7388a

3pointer · 2023-02-14T02:27:42Z

br/pkg/gluetidb/glue_test.go

+	d := dom.DDL()
+	require.NotNil(t, d)
+
+	infos1 := []*model.TableInfo{}


and also add a case that len(infos)==0

add new case in dd7388a

3pointer · 2023-02-14T06:20:26Z

br/pkg/gluetidb/glue.go

+	err = d.BatchCreateTableWithInfo(gs.se, schema, infos, append(cs, ddl.OnExistIgnore)...)
+	failpoint.Inject("RestoreBatchCreateTableEntryTooLarge", func(val failpoint.Value) {
+		if val.(bool) {
+			if len(infos) > 1 {


Suggested change

if len(infos) > 1 {

3pointer · 2023-02-14T06:25:16Z

br/pkg/gluetidb/glue_test.go

+
+	require.NoError(t, err)
+
+	require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/br/pkg/gluetidb/RestoreBatchCreateTableEntryTooLarge"))


can we add a check that id has reused?

fengou1 · 2023-02-14T09:24:05Z

/merge

ti-chi-bot · 2023-02-14T09:24:09Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 4694132

YuJuncen · 2023-02-14T09:38:42Z

/test unit-test

fengou1 · 2023-02-14T09:39:32Z

/test unit-test

fengou1 · 2023-02-14T10:23:02Z

/merge

ti-chi-bot · 2023-02-14T10:23:06Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 594db2b

ti-chi-bot · 2023-02-14T11:06:45Z

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

…gcap#41358) close pingcap#41333

ti-chi-bot · 2023-04-06T02:24:09Z

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

ti-chi-bot · 2023-04-06T02:24:11Z

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

) (#42823) close #41333

fix: reused table id been disabled

50acd22

ti-chi-bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. labels Feb 13, 2023

fengou1 requested a review from 3pointer February 13, 2023 14:04

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 13, 2023

fengou1 requested a review from YuJuncen February 13, 2023 14:04

fengou1 added 2 commits February 13, 2023 22:06

fix: remove unused test help function

139f65f

fix: remove unused help function

ac617af

YuJuncen approved these changes Feb 13, 2023

View reviewed changes

br/pkg/gluetidb/glue_test.go Show resolved Hide resolved

br/pkg/gluetidb/glue_test.go Outdated Show resolved Hide resolved

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 13, 2023

fengou1 and others added 3 commits February 13, 2023 22:51

revert a commit

6e0008f

Update br/pkg/gluetidb/glue_test.go

0d85974

Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>

update bazel and add some comments

effff38

fengou1 added component/br This issue is related to BR of TiDB. and removed do-not-merge/needs-triage-completed labels Feb 13, 2023

fengou1 self-assigned this Feb 13, 2023

fengou1 added the needs-cherry-pick-release-6.6 label Feb 13, 2023

fix: drop test table in case of tables existed.

fc757f1

3pointer reviewed Feb 14, 2023

View reviewed changes

fengou1 added 3 commits February 14, 2023 11:25

fix: remove optional parameter and enhance unit test cases

dd7388a

fix: add compile check and update funciton signature

d114b7c

fix: add a fail point and new unit test

a8dc751

3pointer reviewed Feb 14, 2023

View reviewed changes

fengou1 added 4 commits February 14, 2023 15:37

fix: move failpoint to ddl package

bd2de8a

fix: add reused table id check

31fd37f

fix: add a new failure case

7ff7752

fix: linter issue

a4e8c59

3pointer approved these changes Feb 14, 2023

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 14, 2023

fengou1 added 2 commits February 14, 2023 16:39

fix: remove empty line

84575c9

fix: update bazel

4694132

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 14, 2023

YuJuncen mentioned this pull request Feb 14, 2023

br: reused table id is disabled when restore a brand-new cluster #41402

Merged

12 tasks

fix: update bazel

594db2b

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Feb 14, 2023

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 14, 2023

ti-chi-bot merged commit 41c1250 into pingcap:master Feb 14, 2023

ti-chi-bot mentioned this pull request Feb 14, 2023

br: reused table id is disabled when restore a brand-new cluster (#41358) #41404

Closed

12 tasks

blacktear23 pushed a commit to blacktear23/tidb that referenced this pull request Feb 15, 2023

br: reused table id is disabled when restore a brand-new cluster (pin…

c903e9e

…gcap#41358) close pingcap#41333

ghazalfamilyusa pushed a commit to ghazalfamilyusa/tidb that referenced this pull request Feb 15, 2023

br: reused table id is disabled when restore a brand-new cluster (pin…

0231621

…gcap#41358) close pingcap#41333

3pointer added needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. labels Apr 6, 2023

ti-chi-bot mentioned this pull request Apr 6, 2023

br: reused table id is disabled when restore a brand-new cluster (#41358) #42822

Closed

12 tasks

ti-chi-bot mentioned this pull request Apr 6, 2023

br: reused table id is disabled when restore a brand-new cluster (#41358) #42823

Merged

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Apr 6, 2023

This is an automated cherry-pick of pingcap#41358

62796d9

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 Apr 6, 2023

This is an automated cherry-pick of pingcap#41358

e996e36

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

3pointer mentioned this pull request Apr 6, 2023

br: recursive split batch create table when restore failure on batch create table (#39829) #42775

Merged

12 tasks

ti-chi-bot added a commit that referenced this pull request Apr 10, 2023

br: reused table id is disabled when restore a brand-new cluster (#41358

8a82a0d

) (#42823) close #41333


		require.NoError(t, err)

		require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/br/pkg/gluetidb/RestoreBatchCreateTableEntryTooLarge"))

Conversation

fengou1 commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YuJuncen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

3pointer Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

fengou1 Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

3pointer Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

fengou1 Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

3pointer Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

3pointer Feb 14, 2023

Choose a reason for hiding this comment

Uh oh!

fengou1 commented Feb 14, 2023

Uh oh!

ti-chi-bot commented Feb 14, 2023

Uh oh!

YuJuncen commented Feb 14, 2023

Uh oh!

fengou1 commented Feb 14, 2023

Uh oh!

fengou1 commented Feb 14, 2023

Uh oh!

ti-chi-bot commented Feb 14, 2023

Uh oh!

ti-chi-bot commented Feb 14, 2023

Uh oh!

ti-chi-bot commented Apr 6, 2023

Uh oh!

ti-chi-bot commented Apr 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fengou1 commented Feb 13, 2023 •

edited

Loading

ti-chi-bot commented Feb 13, 2023 •

edited

Loading