syncer: fix the case-sensitive issue #1738

glorv · 2021-05-31T07:46:20Z

What problem does this PR solve?

Fix the bug that case-sensitive can not take effect as expected.

What is changed and how it works?

Check upstream lower_case_table_names setting at bootstrap, do not allow set case-sensitive = true when lower_case_table_names = true
return schema/table name base on lower_case_table_names, return Name.O for 0, and Name.L for 1/2.
Try fix all the persistent data at syncer start if lower_case_table_names = 0

Check List

Tests

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

Code changes

Has persistent data change

Side effects

Breaking backward compatibility
If the cluster is upgrade for a lower version with some upstream lower_case_table_names = 0, we can't rollback to the origin version since some persistent data has been changed.

Related changes

Need to cherry-pick to the release branch
Need to be included in the release note

…sensitive

glorv · 2021-06-02T08:17:09Z

/run-all-tests

glorv · 2021-06-02T12:09:21Z

/build

glorv · 2021-06-03T11:37:30Z

/run-all-tests

glorv · 2021-06-04T09:33:27Z

/run-all-tests

…sensitive

glorv · 2021-06-07T05:51:39Z

/run-all-tests

lance6716

will review soon

lance6716 · 2021-07-16T06:38:52Z

syncer/shardddl/optimist.go

@@ -202,3 +202,26 @@ func (o *Optimist) PendingOperation() *optimism.Operation {
 op := *o.pendingOp
 return &op
 }
+
+// CheckPersistentData check and fix the persistent data.


may address that currently this function is not used because user will meet error at early version if set unsupported case-sensitive.

lance6716 · 2021-07-16T06:42:04Z

syncer/checkpoint.go

@@ -840,6 +843,44 @@ func (cp *RemoteCheckPoint) Load(tctx *tcontext.Context) error {
 return terror.WithScope(terror.DBErrorAdapt(rows.Err(), terror.ErrDBDriverError), terror.ScopeDownstream)
 }

+// CheckAndUpdate check the checkpoint data consistency and try to fix them if possible.
+func (cp *RemoteCheckPoint) CheckAndUpdate(ctx context.Context, schemas map[string]string, tables map[string]map[string]string) error {
+ cp.Lock()


maybe no lock is needed (but it's OK to acquire a lock)

lance6716

rest lgtm

syncer/syncer.go

tests/others_integration.txt

tests/case-sensitive/run.sh

lichunzhu

Will review later

lichunzhu · 2021-07-16T07:23:28Z

chaos/cases/task.go

+ if lcSetting == utils.LCTableNamesMixed {
+ msg := "can not set `case-sensitive = true` when upstream `lower_case_table_names = 2`"
+ log.L().Error(msg, zap.Any("instance", cfg))
+ return nil, errors.New(msg)


Will this make a chaos test fail?

Not sure 🤔

lichunzhu · 2021-07-16T08:07:51Z

pkg/shardddl/optimism/info.go

@@ -344,3 +344,47 @@ func (oldInfo *OldInfo) toInfo() Info {
 TableInfosAfter: []*model.TableInfo{oldInfo.TableInfoAfter},
 }
 }
+
+// CheckDDLInfos try to check and fix all the schema and table names for DDL info.
+func CheckDDLInfos(cli *clientv3.Client, source string, schemaMap map[string]string, talesMap map[string]map[string]string) error {


Suggested change

func CheckDDLInfos(cli *clientv3.Client, source string, schemaMap map[string]string, talesMap map[string]map[string]string) error {

func CheckDDLInfos(cli *clientv3.Client, source string, schemaMap map[string]string, tablesMap map[string]map[string]string) error {

lichunzhu · 2021-07-16T08:16:40Z

pkg/utils/common.go

+// GenTableID generates table ID.
+func GenTableID(schema, table string) (id string, isSchemaOnly bool) {
+ if len(table) == 0 {
+ return fmt.Sprintf("`%s`", schema), true


Will this have a better performance?

Suggested change

return fmt.Sprintf("`%s`", schema), true

return "`" + schema + "`", true

lichunzhu · 2021-07-16T08:16:52Z

pkg/utils/common.go

+ if len(table) == 0 {
+ return fmt.Sprintf("`%s`", schema), true
+ }
+ return fmt.Sprintf("`%s`.`%s`", schema, table), false


lichunzhu · 2021-07-16T08:44:48Z

syncer/checkpoint.go

+ tctx := &tcontext.Context{
+ Logger: log.L(),
+ Ctx: ctx,
+ }


Suggested change

tctx := &tcontext.Context{

Logger: log.L(),

Ctx: ctx,

}

tctx := tcontext.NewContext(ctx, log.L())

lichunzhu · 2021-07-19T05:32:15Z

syncer/online-ddl-tools/online_ddl.go

@@ -65,6 +65,8 @@ type OnlinePlugin interface {
 Clear(tctx *tcontext.Context) error
 // Close closes online ddl plugin
 Close()
+ // CheckAndUpdate try to check and fix the shcema/table case-sensitive issue


Suggested change

// CheckAndUpdate try to check and fix the shcema/table case-sensitive issue

// CheckAndUpdate try to check and fix the schema/table case-sensitive issue

glorv · 2021-07-19T07:19:16Z

/run-all-tests

lichunzhu

LGTM

ti-chi-bot · 2021-07-19T08:39:23Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lance6716
lichunzhu

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.

glorv · 2021-07-19T08:52:51Z

/merge

ti-chi-bot · 2021-07-19T08:52:54Z

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

Commit hash: 0a09c5b

glorv · 2021-07-19T09:50:51Z

/run-all-tests

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

ti-chi-bot · 2021-07-19T10:10:12Z

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

fix syncer case-sensitive issue

b17efdf

ti-chi-bot added the do-not-merge/work-in-progress label May 31, 2021

ti-chi-bot requested review from lance6716 and lichunzhu May 31, 2021 07:46

ti-chi-bot added the size/XXL label May 31, 2021

fix query

a97d7a1

glorv force-pushed the case-sensitive branch from de68ef1 to 2508185 Compare May 31, 2021 12:12

try to fix an unstable test

ac53943

glorv force-pushed the case-sensitive branch from 2508185 to ac53943 Compare June 1, 2021 02:31

lance6716 added needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Jun 1, 2021

lance6716 added this to the v2.0.4 milestone Jun 1, 2021

glorv added 7 commits June 1, 2021 11:21

fix unit test

ffee76b

fix unit test

d7b3385

add case-sensitive test

fd6730e

Merge branch 'case-sensitive' of ssh://github.com/glorv/dm into case-…

620336d

…sensitive

Merge branch 'case-sensitive' of ssh://github.com/glorv/dm into case-…

54bc9df

…sensitive

fmt test

af4f835

Merge branch 'case-sensitive' of ssh://github.com/glorv/dm into case-…

c8932f1

…sensitive

glorv added 2 commits June 3, 2021 17:44

update tidb-tools

4813d88

Merge branch 'master' of ssh://github.com/pingcap/dm into case-sensitive

3de549f

Merge branch 'master' into case-sensitive

1806894

glorv added 2 commits June 4, 2021 17:35

fix go.sum

b4009a3

Merge branch 'case-sensitive' of ssh://github.com/glorv/dm into case-…

9ec36aa

…sensitive

use filter.Apply to avoid the case-sensitive bug

2e867eb

lance6716 reviewed Jul 16, 2021

View reviewed changes

syncer/syncer.go Show resolved Hide resolved

tests/others_integration.txt Outdated Show resolved Hide resolved

tests/case-sensitive/run.sh Outdated Show resolved Hide resolved

lichunzhu reviewed Jul 16, 2021

View reviewed changes

ti-chi-bot added the needs-rebase label Jul 16, 2021

glorv added 2 commits July 19, 2021 10:31

Merge branch 'master' of ssh://github.com/pingcap/dm into case-sensitive

93191e1

resolve comments

553049d

ti-chi-bot removed the needs-rebase label Jul 19, 2021

lance6716 approved these changes Jul 19, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 One reviewer already commented LGTM label Jul 19, 2021

lichunzhu reviewed Jul 19, 2021

View reviewed changes

fix typo

0a09c5b

lichunzhu approved these changes Jul 19, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jul 19, 2021

ti-chi-bot added the status/can-merge label Jul 19, 2021

Merge branch 'master' into case-sensitive

08aa33c

ti-chi-bot merged commit a5a219c into pingcap:master Jul 19, 2021

glorv deleted the case-sensitive branch July 19, 2021 10:09

ti-chi-bot pushed a commit to ti-chi-bot/dm that referenced this pull request Jul 19, 2021

This is an automated cherry-pick of pingcap#1738

9477f57

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

ti-chi-bot mentioned this pull request Jul 19, 2021

syncer: fix the case-sensitive issue (#1738) #1886

Merged

GMHDBJD mentioned this pull request Jul 23, 2021

block-allow-list and other configurations can't work when set uppercase db/table name and case-sensitive = true #1676

Closed

ti-chi-bot added a commit that referenced this pull request Jul 26, 2021

syncer: fix the case-sensitive issue (#1738) (#1886)

1857994

This was referenced Aug 2, 2021

support mixing usage of uppercase and lowercase when upstream lower_case_table_names not 0 #1646

Closed

Support case sensitive upstream more automatically #971

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syncer: fix the case-sensitive issue #1738

syncer: fix the case-sensitive issue #1738

glorv commented May 31, 2021 •

edited

Loading

glorv commented Jun 2, 2021

glorv commented Jun 2, 2021

glorv commented Jun 3, 2021

glorv commented Jun 4, 2021

glorv commented Jun 7, 2021

lance6716 left a comment

lance6716 Jul 16, 2021

lance6716 Jul 16, 2021

lance6716 left a comment

lichunzhu left a comment

lichunzhu Jul 16, 2021

glorv Jul 19, 2021

lichunzhu Jul 16, 2021

lichunzhu Jul 16, 2021

lichunzhu Jul 16, 2021

lichunzhu Jul 16, 2021

lichunzhu Jul 19, 2021

glorv commented Jul 19, 2021

lichunzhu left a comment

ti-chi-bot commented Jul 19, 2021

glorv commented Jul 19, 2021

ti-chi-bot commented Jul 19, 2021

glorv commented Jul 19, 2021

ti-chi-bot commented Jul 19, 2021

	func CheckDDLInfos(cli *clientv3.Client, source string, schemaMap map[string]string, talesMap map[string]map[string]string) error {
	func CheckDDLInfos(cli *clientv3.Client, source string, schemaMap map[string]string, tablesMap map[string]map[string]string) error {

	return fmt.Sprintf("`%s`", schema), true
	return "`" + schema + "`", true

	// CheckAndUpdate try to check and fix the shcema/table case-sensitive issue
	// CheckAndUpdate try to check and fix the schema/table case-sensitive issue

syncer: fix the case-sensitive issue #1738

syncer: fix the case-sensitive issue #1738

Conversation

glorv commented May 31, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

glorv commented Jun 2, 2021

glorv commented Jun 2, 2021

glorv commented Jun 3, 2021

glorv commented Jun 4, 2021

glorv commented Jun 7, 2021

lance6716 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lance6716 left a comment

Choose a reason for hiding this comment

lichunzhu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glorv commented Jul 19, 2021

lichunzhu left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Jul 19, 2021

glorv commented Jul 19, 2021

ti-chi-bot commented Jul 19, 2021

glorv commented Jul 19, 2021

ti-chi-bot commented Jul 19, 2021

glorv commented May 31, 2021 •

edited

Loading