br: fix compatibility issue with concurrent ddl #36474

3pointer · 2022-07-22T07:57:54Z

What problem does this PR solve?

Issue Number: close #36453

Problem Summary:
After concurrent DDL feature implemented. v6.2 cluster will use a new way(ddl table) to execute ddls. but when using v6.2 br to restore cluster to v6.1 doesn't have such ddl tables.

What is changed and how it works?

This PR add a check. when using br (> v6.2) restore cluster (< v6.1). br will disable the switch of concurrent ddl.

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-07-22T07:57:55Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
wjhuang2016

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.

xiongjiwei · 2022-07-22T08:23:02Z

br/pkg/version/version.go

+// CheckVersionForDDL checks whether we use queue or table to execute ddl during restore.
+func CheckVersionForDDL(s *metapb.Store, tikvVersion *semver.Version) error {
+	// use tikvVersion instead of tidbVersion since br doesn't have mysql client to connect tidb.
+	if tikvVersion.Major < 6 || (tikvVersion.Major == 6 && tikvVersion.Minor <= 1) {


can use compare method

xiongjiwei · 2022-07-22T08:23:23Z

br/pkg/version/version.go

+	// use tikvVersion instead of tidbVersion since br doesn't have mysql client to connect tidb.
+	if tikvVersion.Major < 6 || (tikvVersion.Major == 6 && tikvVersion.Minor <= 1) {
+		log.Info("detected the old version of tidb cluster. set enable concurrent ddl to false")
+		variable.EnableConcurrentDDL.Store(false)


no need to set it back?

yes, this variable only works for br when br exists.

D3Hunter

lgtm

3pointer · 2022-07-22T08:36:09Z

/hold

br/pkg/version/version.go

3pointer · 2022-07-22T09:05:49Z

/unhold

3pointer · 2022-07-22T10:04:24Z

/run-check_dev_2

3pointer · 2022-07-22T10:04:29Z

/run-mysql-test

3pointer · 2022-07-22T10:04:35Z

/run-unit-test

3pointer · 2022-07-23T05:40:25Z

/run-unit-tests

sre-bot · 2022-07-23T06:13:25Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/3f99fa2a18859ca53fb9340c797c03b56865adc0

3pointer · 2022-07-23T06:28:32Z

/run-integration-br-tests

3pointer · 2022-07-23T08:33:01Z

/run-integration-br-tests

3pointer · 2022-07-23T09:17:55Z

/merge

ti-chi-bot · 2022-07-23T09:17:59Z

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

Commit hash: 20393e2

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2022-07-23T09:48:00Z

cherry pick to release-6.1 in PR #36488

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2022-07-23T09:49:15Z

cherry pick to release-6.2 in PR #36489

sre-bot · 2022-07-23T10:56:19Z

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-common-test	🟥 failed 1, success 10, total 11	1 hr 3 min	New failing
idc-jenkins-ci/integration-cdc-test	🟢 all 36 tests passed	25 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 12 tests passed	17 min	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	7 min 46 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	7 min 26 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	6 min 47 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	5 min 38 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 36 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 48 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

…rimary-key * upstream/master: (104 commits) br: fix compatibility issue with concurrent ddl (pingcap#36474) infoschema: fix PromQL for `tidb_distsql_copr_cache` (pingcap#36450) test: stabilize TestTopSQLCPUProfile (pingcap#36468) parser: add support of 'ADMIN SHOW DDL JOB QUERIES LIMIT m OFFSET n' transferring to AST (pingcap#36285) *: enable flaky test for all test (pingcap#36385) expression: fix return type of agg func `bit_or` when handling varbinary column (pingcap#36415) executor: fix aggregating enum zero value gets different results from mysql (pingcap#36208) server: skip check tiflash version (pingcap#36451) *: Minor update to SECURITY.md to improved clarity (pingcap#36346) table partition: add telemetry for partition table (pingcap#36204) ddl: invalid multiple MAXVALUE partitions (pingcap#36329) (pingcap#36345) planner: Fixed `Merge` hint in nested CTE (pingcap#36432) metric: impove concurrency ddl metrics (pingcap#36405) planner: add more test cases for leading outer join (pingcap#36409) ddl: only set concurrent variable if no error (pingcap#36437) ddl: fix update panic in the middle of multi-schema change (pingcap#36421) session: Mising OptimizeWithPlanAndThenWarmUp in prepare-execute path (pingcap#36347) executor,metrics: add a metric for observing execution phases (pingcap#35906) br: unified docker image align with tidb (pingcap#36016) ddl: skip to close nil sessPool (pingcap#36425) ...

close #36453

br: fix compatibility issue with concurrent ddl

3e0fbbd

3pointer added 2 commits July 22, 2022 16:01

add test

3b6518d

add log

dcb8edb

update

e019170

3pointer marked this pull request as ready for review July 22, 2022 08:16

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 22, 2022

xiongjiwei reviewed Jul 22, 2022

View reviewed changes

address comment

70b7191

D3Hunter approved these changes Jul 22, 2022

View reviewed changes

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

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

xiongjiwei reviewed Jul 22, 2022

View reviewed changes

br/pkg/version/version.go Outdated Show resolved Hide resolved

wjhuang2016 approved these changes Jul 22, 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 Jul 22, 2022

address comment

b31f628

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

upadte test

175fd85

3pointer force-pushed the fix_compatibility_issue_with_ddl branch from 3532d7f to 175fd85 Compare July 22, 2022 10:09

ti-chi-bot added the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Jul 22, 2022

fix test

5c30510

3pointer force-pushed the fix_compatibility_issue_with_ddl branch from 81da308 to 5c30510 Compare July 23, 2022 06:05

fmt

0169170

wjhuang2016 removed the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Jul 23, 2022

fix unstable rawkv test

20393e2

ti-chi-bot added the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Jul 23, 2022

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

Merge branch 'master' into fix_compatibility_issue_with_ddl

3f99fa2

ti-chi-bot merged commit 28a8ffc into pingcap:master Jul 23, 2022

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jul 23, 2022

cherry pick pingcap#36474 to release-6.1

3cf4ae7

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Jul 23, 2022

br: fix compatibility issue with concurrent ddl (#36474) #36488

Closed

12 tasks

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jul 23, 2022

cherry pick pingcap#36474 to release-6.2

cfa23a6

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Jul 23, 2022

br: fix compatibility issue with concurrent ddl (#36474) #36489

Merged

12 tasks

ti-chi-bot pushed a commit that referenced this pull request Jul 25, 2022

br: fix compatibility issue with concurrent ddl (#36474) (#36489)

2fde17e

close #36453

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

br: fix compatibility issue with concurrent ddl #36474

br: fix compatibility issue with concurrent ddl #36474

3pointer commented Jul 22, 2022 •

edited

Loading

ti-chi-bot commented Jul 22, 2022 •

edited

Loading

xiongjiwei Jul 22, 2022

xiongjiwei Jul 22, 2022

3pointer Jul 22, 2022 •

edited

Loading

D3Hunter left a comment

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 23, 2022

sre-bot commented Jul 23, 2022 •

edited

Loading

3pointer commented Jul 23, 2022

3pointer commented Jul 23, 2022

3pointer commented Jul 23, 2022

ti-chi-bot commented Jul 23, 2022

ti-srebot commented Jul 23, 2022

ti-srebot commented Jul 23, 2022

sre-bot commented Jul 23, 2022

br: fix compatibility issue with concurrent ddl #36474

br: fix compatibility issue with concurrent ddl #36474

Conversation

3pointer commented Jul 22, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jul 22, 2022 • edited Loading

xiongjiwei Jul 22, 2022

Choose a reason for hiding this comment

xiongjiwei Jul 22, 2022

Choose a reason for hiding this comment

3pointer Jul 22, 2022 • edited Loading

Choose a reason for hiding this comment

D3Hunter left a comment

Choose a reason for hiding this comment

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 22, 2022

3pointer commented Jul 23, 2022

sre-bot commented Jul 23, 2022 • edited Loading

3pointer commented Jul 23, 2022

3pointer commented Jul 23, 2022

3pointer commented Jul 23, 2022

ti-chi-bot commented Jul 23, 2022

ti-srebot commented Jul 23, 2022

ti-srebot commented Jul 23, 2022

sre-bot commented Jul 23, 2022

TiDB MergeCI notify

3pointer commented Jul 22, 2022 •

edited

Loading

ti-chi-bot commented Jul 22, 2022 •

edited

Loading

3pointer Jul 22, 2022 •

edited

Loading

sre-bot commented Jul 23, 2022 •

edited

Loading