*: support 2-phases flashback cluster #38164

Defined2014 · 2022-09-26T08:16:12Z

What problem does this PR solve?

Issue Number: close #38554, #37961

Problem Summary:

What is changed and how it works?

change flashback to two phases, first stage lock all regions, second stage do flashback.
close all cross flashback cluster connection.

Above two reasons, we could remove many useless codes in Flashback. Like history flashback time ranges, auto analyze max time etc.

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 · 2022-09-26T08:16:13Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

bb7133
xhebox

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.

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

Co-authored-by: bb7133 <[email protected]>

Defined2014 · 2022-10-26T02:38:08Z

Change flashback to two phases

This is not needed before 'flashback cluster' is released as a feature. Please change it to 'None'

Done, PTAL @bb7133

bb7133

LGTM

xhebox

LGTM, two questions

infoschema/builder.go

domain/schema_validator.go

xhebox · 2022-10-26T07:18:13Z

/merge

ti-chi-bot · 2022-10-26T07:18:17Z

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

Commit hash: 06bdda7

zimulala · 2022-10-26T07:37:37Z

ddl/cluster.go

+		// Split region by keyRanges, make sure no unrelated key ranges be locked.
+		splitRegionsByKeyRanges(d, keyRanges)
+		totalRegions.Store(0)
+		for _, ranges := range keyRanges {


Suggested change

for _, ranges := range keyRanges {

for _, range := range keyRanges {

zimulala · 2022-10-26T07:41:14Z

ddl/cluster.go

+			endKey = rangeEndKey
+		}
+
+		logutil.BgLogger().Info("send prepare flashback request", zap.Uint64("region_id", loc.Region.GetID()),


Suggested change

logutil.BgLogger().Info("send prepare flashback request", zap.Uint64("region_id", loc.Region.GetID()),

logutil.BgLogger().Info("[ddl] send prepare flashback request", zap.Uint64("region_id", loc.Region.GetID()),

zimulala · 2022-10-26T07:42:32Z

ddl/cluster.go

+
+		endKey := loc.EndKey
+		isLast := len(endKey) == 0 || (len(rangeEndKey) > 0 && bytes.Compare(endKey, rangeEndKey) >= 0)
+		// If it is the last region


Suggested change

// If it is the last region

// If it is the last region.

zimulala · 2022-10-26T07:47:18Z

ddl/cluster.go

@@ -422,16 +565,25 @@ func (w *worker) onFlashbackCluster(d *ddlCtx, t *meta.Meta, job *model.Job) (ve
 			job.SchemaState = model.StatePublic
 			return ver, nil
 		}
-
-		keyRanges, err := GetFlashbackKeyRanges(sess, tablecodec.EncodeTablePrefix(0))
+		keyRanges, err := GetFlashbackKeyRanges(sess)
 		if err != nil {
 			return ver, errors.Trace(err)
 		}

 		for _, ranges := range keyRanges {


Suggested change

for _, ranges := range keyRanges {

for _, range := range keyRanges {

zimulala · 2022-10-26T07:49:24Z

ddl/cluster.go

+					// use commitTS - 1 as startTS, make sure it less than commitTS.
+					stats, err := sendFlashbackToVersionRPC(ctx, d.store.(tikv.Storage), flashbackTS, commitTS-1, commitTS, r)
+					completedRegions.Add(uint64(stats.CompletedRegions))
+					logutil.BgLogger().Info("flashback cluster stats",


Suggested change

logutil.BgLogger().Info("flashback cluster stats",

logutil.BgLogger().Info("[ddl] flashback cluster stats",

zimulala · 2022-10-26T07:49:38Z

ddl/cluster.go

-			if err = flashbackToVersion(context.Background(), d, flashbackTS, ranges.StartKey, ranges.EndKey); err != nil {
+			if err = flashbackToVersion(d.ctx, d,
+				func(ctx context.Context, r tikvstore.KeyRange) (rangetask.TaskStat, error) {
+					// use commitTS - 1 as startTS, make sure it less than commitTS.


Suggested change

// use commitTS - 1 as startTS, make sure it less than commitTS.

// Use commitTS - 1 as startTS, make sure it less than commitTS.

zimulala · 2022-10-26T07:57:24Z

ddl/cluster.go


-	return keyRanges, nil
+func prepareFlashbackToVersion(


It seems that this function code is the same as flashbackToVersion. Is it duplicated?

xhebox · 2022-10-26T08:04:22Z

/hold

xhebox · 2022-10-26T08:28:35Z

/unhold

Defined2014 · 2022-10-26T08:38:20Z

/unhold

Defined2014 · 2022-10-26T08:38:29Z

/merge

ti-chi-bot · 2022-10-26T08:38:33Z

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

Commit hash: 45054eb

Defined2014 · 2022-10-26T08:38:35Z

/run-build

sre-bot · 2022-10-26T09:37:29Z

TiDB MergeCI notify

✅ Well Done! New fixed [3] after this pr merged.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/common-test	✅ all 11 tests passed	10 min	Fixed
idc-jenkins-ci-tidb/sqllogic-test-2	✅ all 28 tests passed	6 min 3 sec	Fixed
idc-jenkins-ci-tidb/sqllogic-test-1	✅ all 26 tests passed	5 min 34 sec	Fixed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	29 min	Existing passed
idc-jenkins-ci/integration-cdc-test	🟢 all 38 tests passed	21 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 17 tests passed	17 min	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	6 min 34 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 14 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	3 min 4 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

Defined2014 added 9 commits September 20, 2022 19:12

*: add startTS, commitTS and close auto analyze for flashback

4970a67

improve error msg

e9e588f

update bazel

4e7960d

fix teset

e6b48ad

Merge branch 'master' into flashback-auto

f58a8ac

delete useless code

115adb5

kill non-flashback cluster query

fdc6653

update test

5e5ccd7

fix ut

6e4922d

fix test

12fa7a0

Defined2014 changed the title ~~[WIP] Flashback 2 phase~~ [WIP] *: support 2-phases flashback cluster Sep 27, 2022

ti-chi-bot removed the do-not-merge/invalid-title label Sep 27, 2022

Defined2014 added 4 commits September 27, 2022 11:10

support tiflash

521a80b

Merge branch 'master' into flashback-2-phase

feb6639

delete useless param

b15b8c3

Merge branch 'master' into flashback-2-phase

20892f0

Defined2014 requested a review from a team as a code owner September 29, 2022 12:19

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 29, 2022

Defined2014 added 2 commits October 8, 2022 11:53

update kvproto and client-go

135e67c

Merge branch 'master' into flashback-2-phase

f9ea4db

Defined2014 force-pushed the flashback-2-phase branch from 0105f54 to f9ea4db Compare October 8, 2022 04:59

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

update DEPS

135fe22

ti-chi-bot removed the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 26, 2022

Defined2014 and others added 2 commits October 26, 2022 10:16

Update ddl/cluster.go

c36f7a9

Co-authored-by: bb7133 <[email protected]>

update test

06bdda7

bb7133 approved these changes Oct 26, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 26, 2022

xhebox reviewed Oct 26, 2022

View reviewed changes

infoschema/builder.go Show resolved Hide resolved

domain/schema_validator.go Show resolved Hide resolved

xhebox approved these changes Oct 26, 2022

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 Oct 26, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 26, 2022

Merge branch 'master' into flashback-2-phase

acac5d6

zimulala reviewed Oct 26, 2022

View reviewed changes

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

follow comments

45054eb

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Oct 26, 2022

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

zimulala approved these changes Oct 26, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 26, 2022

ti-chi-bot merged commit 4bdd20b into pingcap:master Oct 26, 2022

Defined2014 deleted the flashback-2-phase branch October 26, 2022 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: support 2-phases flashback cluster #38164

*: support 2-phases flashback cluster #38164

Defined2014 commented Sep 26, 2022 •

edited

Loading

ti-chi-bot commented Sep 26, 2022 •

edited

Loading

Defined2014 commented Oct 26, 2022

bb7133 left a comment

xhebox left a comment

xhebox commented Oct 26, 2022

ti-chi-bot commented Oct 26, 2022

zimulala Oct 26, 2022

zimulala Oct 26, 2022

zimulala Oct 26, 2022

zimulala Oct 26, 2022

zimulala Oct 26, 2022

zimulala Oct 26, 2022

zimulala Oct 26, 2022

xhebox commented Oct 26, 2022

xhebox commented Oct 26, 2022

Defined2014 commented Oct 26, 2022

Defined2014 commented Oct 26, 2022

ti-chi-bot commented Oct 26, 2022

Defined2014 commented Oct 26, 2022

sre-bot commented Oct 26, 2022

	for _, ranges := range keyRanges {
	for _, range := range keyRanges {

	logutil.BgLogger().Info("send prepare flashback request", zap.Uint64("region_id", loc.Region.GetID()),
	logutil.BgLogger().Info("[ddl] send prepare flashback request", zap.Uint64("region_id", loc.Region.GetID()),

	logutil.BgLogger().Info("flashback cluster stats",
	logutil.BgLogger().Info("[ddl] flashback cluster stats",

	// use commitTS - 1 as startTS, make sure it less than commitTS.
	// Use commitTS - 1 as startTS, make sure it less than commitTS.

*: support 2-phases flashback cluster #38164

*: support 2-phases flashback cluster #38164

Conversation

Defined2014 commented Sep 26, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Sep 26, 2022 • edited Loading

Defined2014 commented Oct 26, 2022

bb7133 left a comment

Choose a reason for hiding this comment

xhebox left a comment

Choose a reason for hiding this comment

xhebox commented Oct 26, 2022

ti-chi-bot commented Oct 26, 2022

zimulala Oct 26, 2022

Choose a reason for hiding this comment

zimulala Oct 26, 2022

Choose a reason for hiding this comment

zimulala Oct 26, 2022

Choose a reason for hiding this comment

zimulala Oct 26, 2022

Choose a reason for hiding this comment

zimulala Oct 26, 2022

Choose a reason for hiding this comment

zimulala Oct 26, 2022

Choose a reason for hiding this comment

zimulala Oct 26, 2022

Choose a reason for hiding this comment

xhebox commented Oct 26, 2022

xhebox commented Oct 26, 2022

Defined2014 commented Oct 26, 2022

Defined2014 commented Oct 26, 2022

ti-chi-bot commented Oct 26, 2022

Defined2014 commented Oct 26, 2022

sre-bot commented Oct 26, 2022

TiDB MergeCI notify

Defined2014 commented Sep 26, 2022 •

edited

Loading

ti-chi-bot commented Sep 26, 2022 •

edited

Loading