br: modify collate.newCollationEnabled according to the config of the cluster #39173

MoCuishle28 · 2022-11-16T03:39:44Z

What problem does this PR solve?

Issue Number: close #39150

Problem Summary:
The collate.newCollationEnabled variable is set to 1 when the collate package is initialized. When using this package, this value is not modified according to the config of the TiDB cluster (i.e., new_collations_enabled_on_first_bootstrap).

As a result, the user sets new_collations_enabled_on_first_bootstrap to false in the TiDB cluster, and it is still treated as true when br is executed.

What is changed and how it works?

Modify collate.newCollationEnabled according to the config of the cluster.

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

Manual test

create a cluster with tidb config new_collations_enabled_on_first_bootstrap: false.
create a table with collation 'utf8mb4_0900_ai_ci': create table books (id bigint auto_increment,name varchar(64), primary key (id)) CHARSET utf8mb4 COLLATE utf8mb4_0900_ai_ci;
insert several records to the table.
execute a full backup
create another cluster tidb config new_collations_enabled_on_first_bootstrap: false.
restore the backup to the new cluster => success

Release note

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

Fix a bug that: "Error Unsupported collation when new collation is enabled: 'utf8mb4_0900_ai_ci'" even if new_collation_enabled is false.

ti-chi-bot · 2022-11-16T03:39:46Z

[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.

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

MoCuishle28 · 2022-11-16T04:49:18Z

Due to the cycle import caused by importing the br/pkg/gluetidb package when writing unit tests, we moved the CheckNewCollationEnable func to the br/pkg/restore package and moved the const tidbNewCollationEnabled to the br/pkg/utils package.

MoCuishle28 · 2022-11-16T06:02:43Z

/run-check-dev-2

MoCuishle28 · 2022-11-16T09:19:09Z

/component br

WangLe1321

LGTM

ti-chi-bot · 2022-11-18T06:10:54Z

@WangLe1321: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

LGTM

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 ti-community-infra/tichi repository.

br/pkg/restore/client.go

YuJuncen

rest lgtm

br/pkg/restore/client.go

3pointer · 2022-11-22T05:29:19Z

/merge

ti-chi-bot · 2022-11-22T05:29:22Z

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

Commit hash: 1bbd49cc88fc41906c6d486bd4277e50a7db8798

MoCuishle28 · 2022-11-22T05:46:48Z

/run-check-dev-2

MoCuishle28 · 2022-11-22T05:57:57Z

/run-check-dev-2

MoCuishle28 · 2022-11-22T06:18:49Z

/run-unit-test

MoCuishle28 · 2022-11-22T06:34:15Z

/run-check-dev-2

MoCuishle28 · 2022-11-22T08:23:52Z

/run-check-dev-2
/run-unit-test

MoCuishle28 · 2022-11-22T08:26:30Z

/run-check-dev-2

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

3pointer · 2022-11-22T09:46:06Z

/merge

ti-chi-bot · 2022-11-22T09:46:10Z

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

Commit hash: 40e3788

ti-chi-bot · 2022-11-22T11:20:42Z

In response to a cherrypick label: new pull request created: #39319.

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

sre-bot · 2022-11-22T11:45:44Z

TiDB MergeCI notify

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

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/mybatis-test	🔴 failed 1, success 0, total 1	11 min	Existing failure
idc-jenkins-ci/integration-cdc-test	✅ all 39 tests passed	20 min	Fixed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 17 tests passed	19 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 11 tests passed	10 min	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	7 min 5 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	5 min 56 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	5 min 4 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	4 min 25 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 44 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

… cluster (#39173) (#39319) close #39150

ti-chi-bot added do-not-merge/needs-triage-completed release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 16, 2022

MoCuishle28 force-pushed the issue-39150 branch from f95350e to 8e77fed Compare November 16, 2022 04:35

MoCuishle28 changed the title ~~br: modify newCollationEnabled according to the config of the cluster~~ br: modify collate.newCollationEnabled according to the config of the cluster Nov 16, 2022

MoCuishle28 force-pushed the issue-39150 branch from 8e77fed to f8945ab Compare November 16, 2022 05:32

ti-chi-bot added component/br This issue is related to BR of TiDB. and removed do-not-merge/needs-triage-completed labels Nov 16, 2022

WangLe1321 approved these changes Nov 18, 2022

View reviewed changes

3pointer reviewed Nov 18, 2022

View reviewed changes

br/pkg/restore/client.go Show resolved Hide resolved

3pointer reviewed Nov 18, 2022

View reviewed changes

br/pkg/restore/client.go Show resolved Hide resolved

3pointer approved these changes Nov 21, 2022

View reviewed changes

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

YuJuncen approved these changes Nov 22, 2022

View reviewed changes

br/pkg/restore/client.go Outdated Show resolved Hide resolved

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 Nov 22, 2022

MoCuishle28 force-pushed the issue-39150 branch from f8945ab to c6d947c Compare November 22, 2022 04:05

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

MoCuishle28 force-pushed the issue-39150 branch from c6d947c to 1bbd49c Compare November 22, 2022 05:24

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

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

MoCuishle28 force-pushed the issue-39150 branch from 081f8f2 to 54875a6 Compare November 22, 2022 09:39

ti-chi-bot removed status/can-merge Indicates a PR has been approved by a committer. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 22, 2022

MoCuishle28 added 6 commits November 22, 2022 17:43

debug

6564b1c

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

test

64c4f25

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

br: modify newCollationEnabled according to the config of the cluster

39c8a08

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

br: modify newCollationEnabled according to the config of the cluster

71c8846

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

br: modify newCollationEnabled according to the config of the cluster

d4b0369

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

br: modify newCollationEnabled according to the config of the cluster

40e3788

Signed-off-by: zhanggaoming <gaoming.zhang@pingcap.com>

MoCuishle28 force-pushed the issue-39150 branch from 54875a6 to 40e3788 Compare November 22, 2022 09:44

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

ti-chi-bot added 3 commits November 22, 2022 18:04

Merge branch 'master' into issue-39150

bff5a4a

Merge branch 'master' into issue-39150

92bd264

Merge branch 'master' into issue-39150

78fd439

ti-chi-bot merged commit 84703ef into pingcap:master Nov 22, 2022

ti-chi-bot mentioned this pull request Nov 22, 2022

br: modify collate.newCollationEnabled according to the config of the cluster (#39173) #39319

Merged

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 22, 2022

This is an automated cherry-pick of pingcap#39173

4c09f2e

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

ti-chi-bot added a commit that referenced this pull request Nov 25, 2022

br: modify collate.newCollationEnabled according to the config of the…

6a99800

… cluster (#39173) (#39319) close #39150

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

br: modify collate.newCollationEnabled according to the config of the cluster #39173

br: modify collate.newCollationEnabled according to the config of the cluster #39173

MoCuishle28 commented Nov 16, 2022 •

edited

Loading

ti-chi-bot commented Nov 16, 2022 •

edited

Loading

MoCuishle28 commented Nov 16, 2022

MoCuishle28 commented Nov 16, 2022

MoCuishle28 commented Nov 16, 2022

WangLe1321 left a comment

ti-chi-bot commented Nov 18, 2022

YuJuncen left a comment

3pointer commented Nov 22, 2022

ti-chi-bot commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

3pointer commented Nov 22, 2022

ti-chi-bot commented Nov 22, 2022

ti-chi-bot commented Nov 22, 2022

sre-bot commented Nov 22, 2022

br: modify collate.newCollationEnabled according to the config of the cluster #39173

br: modify collate.newCollationEnabled according to the config of the cluster #39173

Conversation

MoCuishle28 commented Nov 16, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Manual test

Release note

ti-chi-bot commented Nov 16, 2022 • edited Loading

MoCuishle28 commented Nov 16, 2022

MoCuishle28 commented Nov 16, 2022

MoCuishle28 commented Nov 16, 2022

WangLe1321 left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Nov 18, 2022

YuJuncen left a comment

Choose a reason for hiding this comment

3pointer commented Nov 22, 2022

ti-chi-bot commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

MoCuishle28 commented Nov 22, 2022

3pointer commented Nov 22, 2022

ti-chi-bot commented Nov 22, 2022

ti-chi-bot commented Nov 22, 2022

sre-bot commented Nov 22, 2022

TiDB MergeCI notify

MoCuishle28 commented Nov 16, 2022 •

edited

Loading

ti-chi-bot commented Nov 16, 2022 •

edited

Loading