From c733d4c9aa7b4b2968489a0c2f0eb0e247cc0862 Mon Sep 17 00:00:00 2001 From: 3pointer Date: Tue, 2 Jan 2024 12:18:31 +0800 Subject: [PATCH 1/3] This is an automated cherry-pick of #49579 Signed-off-by: ti-chi-bot --- br/pkg/restore/client.go | 35 +++++++++++++++++++++++++++++++++-- br/pkg/restore/client_test.go | 10 ++++++++-- br/pkg/task/restore.go | 2 +- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/br/pkg/restore/client.go b/br/pkg/restore/client.go index c8f2db94a19f6..58a7ef6297f1c 100644 --- a/br/pkg/restore/client.go +++ b/br/pkg/restore/client.go @@ -2880,6 +2880,7 @@ func CheckNewCollationEnable( g glue.Glue, storage kv.Storage, CheckRequirements bool, +<<<<<<< HEAD ) error { if backupNewCollationEnable == "" { if CheckRequirements { @@ -2893,15 +2894,19 @@ func CheckNewCollationEnable( return nil } +======= +) (bool, error) { +>>>>>>> 49484b19661 (restore: correct new collation when "--check-requirements=false" (#49579)) se, err := g.CreateSession(storage) if err != nil { - return errors.Trace(err) + return false, errors.Trace(err) } newCollationEnable, err := se.GetGlobalVariable(utils.GetTidbNewCollationEnabled()) if err != nil { - return errors.Trace(err) + return false, errors.Trace(err) } +<<<<<<< HEAD if !strings.EqualFold(backupNewCollationEnable, newCollationEnable) { return errors.Annotatef(berrors.ErrUnknown, @@ -2909,14 +2914,40 @@ func CheckNewCollationEnable( backupNewCollationEnable, newCollationEnable) } +======= +>>>>>>> 49484b19661 (restore: correct new collation when "--check-requirements=false" (#49579)) // collate.newCollationEnabled is set to 1 when the collate package is initialized, // so we need to modify this value according to the config of the cluster // before using the collate package. enabled := newCollationEnable == "True" // modify collate.newCollationEnabled according to the config of the cluster collate.SetNewCollationEnabledForTest(enabled) +<<<<<<< HEAD log.Info("set new_collation_enabled", zap.Bool("new_collation_enabled", enabled)) return nil +======= + log.Info(fmt.Sprintf("set %s", utils.TidbNewCollationEnabled), zap.Bool("new_collation_enabled", enabled)) + + if backupNewCollationEnable == "" { + if CheckRequirements { + return enabled, errors.Annotatef(berrors.ErrUnknown, + "the value '%s' not found in backupmeta. "+ + "you can use \"SELECT VARIABLE_VALUE FROM mysql.tidb WHERE VARIABLE_NAME='%s';\" to manually check the config. "+ + "if you ensure the value '%s' in backup cluster is as same as restore cluster, use --check-requirements=false to skip this check", + utils.TidbNewCollationEnabled, utils.TidbNewCollationEnabled, utils.TidbNewCollationEnabled) + } + log.Warn(fmt.Sprintf("the config '%s' is not in backupmeta", utils.TidbNewCollationEnabled)) + return enabled, nil + } + + if !strings.EqualFold(backupNewCollationEnable, newCollationEnable) { + return enabled, errors.Annotatef(berrors.ErrUnknown, + "the config '%s' not match, upstream:%v, downstream: %v", + utils.TidbNewCollationEnabled, backupNewCollationEnable, newCollationEnable) + } + + return enabled, nil +>>>>>>> 49484b19661 (restore: correct new collation when "--check-requirements=false" (#49579)) } type waitTiFlashBackoffer struct { diff --git a/br/pkg/restore/client_test.go b/br/pkg/restore/client_test.go index d917993391e46..20caa8ed5fe2f 100644 --- a/br/pkg/restore/client_test.go +++ b/br/pkg/restore/client_test.go @@ -1839,19 +1839,25 @@ func TestCheckNewCollationEnable(t *testing.T) { CheckRequirements: true, isErr: true, }, + { + backupMeta: &backuppb.BackupMeta{NewCollationsEnabled: ""}, + newCollationEnableInCluster: "False", + CheckRequirements: false, + isErr: false, + }, } for i, ca := range caseList { g := &gluetidb.MockGlue{ GlobalVars: map[string]string{"new_collation_enabled": ca.newCollationEnableInCluster}, } - err := restore.CheckNewCollationEnable(ca.backupMeta.GetNewCollationsEnabled(), g, nil, ca.CheckRequirements) - + enabled, err := restore.CheckNewCollationEnable(ca.backupMeta.GetNewCollationsEnabled(), g, nil, ca.CheckRequirements) t.Logf("[%d] Got Error: %v\n", i, err) if ca.isErr { require.Error(t, err) } else { require.NoError(t, err) } + require.Equal(t, ca.newCollationEnableInCluster == "True", enabled) } } diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index a6dab1c9d2729..fa9f60889852e 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -584,7 +584,7 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf return errors.Trace(versionErr) } } - if err = restore.CheckNewCollationEnable(backupMeta.GetNewCollationsEnabled(), g, mgr.GetStorage(), cfg.CheckRequirements); err != nil { + if _, err = restore.CheckNewCollationEnable(backupMeta.GetNewCollationsEnabled(), g, mgr.GetStorage(), cfg.CheckRequirements); err != nil { return errors.Trace(err) } From 28f6aa850d75bb0588b0faf6a373929c4695f9cf Mon Sep 17 00:00:00 2001 From: 3pointer Date: Wed, 24 Jan 2024 19:42:58 +0800 Subject: [PATCH 2/3] resolve conflict --- br/pkg/restore/client.go | 47 +++++++--------------------------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/br/pkg/restore/client.go b/br/pkg/restore/client.go index 58a7ef6297f1c..440f0c30dcd0b 100644 --- a/br/pkg/restore/client.go +++ b/br/pkg/restore/client.go @@ -2880,23 +2880,7 @@ func CheckNewCollationEnable( g glue.Glue, storage kv.Storage, CheckRequirements bool, -<<<<<<< HEAD -) error { - if backupNewCollationEnable == "" { - if CheckRequirements { - return errors.Annotatef(berrors.ErrUnknown, - "the config 'new_collations_enabled_on_first_bootstrap' not found in backupmeta. "+ - "you can use \"show config WHERE name='new_collations_enabled_on_first_bootstrap';\" to manually check the config. "+ - "if you ensure the config 'new_collations_enabled_on_first_bootstrap' in backup cluster is as same as restore cluster, "+ - "use --check-requirements=false to skip this check") - } - log.Warn("the config 'new_collations_enabled_on_first_bootstrap' is not in backupmeta") - return nil - } - -======= ) (bool, error) { ->>>>>>> 49484b19661 (restore: correct new collation when "--check-requirements=false" (#49579)) se, err := g.CreateSession(storage) if err != nil { return false, errors.Trace(err) @@ -2906,48 +2890,33 @@ func CheckNewCollationEnable( if err != nil { return false, errors.Trace(err) } -<<<<<<< HEAD - - if !strings.EqualFold(backupNewCollationEnable, newCollationEnable) { - return errors.Annotatef(berrors.ErrUnknown, - "the config 'new_collations_enabled_on_first_bootstrap' not match, upstream:%v, downstream: %v", - backupNewCollationEnable, newCollationEnable) - } - -======= ->>>>>>> 49484b19661 (restore: correct new collation when "--check-requirements=false" (#49579)) // collate.newCollationEnabled is set to 1 when the collate package is initialized, // so we need to modify this value according to the config of the cluster // before using the collate package. enabled := newCollationEnable == "True" // modify collate.newCollationEnabled according to the config of the cluster collate.SetNewCollationEnabledForTest(enabled) -<<<<<<< HEAD - log.Info("set new_collation_enabled", zap.Bool("new_collation_enabled", enabled)) - return nil -======= - log.Info(fmt.Sprintf("set %s", utils.TidbNewCollationEnabled), zap.Bool("new_collation_enabled", enabled)) + log.Info("set new_collations_enabled_on_first_bootstrap", zap.Bool("new_collation_enabled", enabled)) if backupNewCollationEnable == "" { if CheckRequirements { return enabled, errors.Annotatef(berrors.ErrUnknown, - "the value '%s' not found in backupmeta. "+ - "you can use \"SELECT VARIABLE_VALUE FROM mysql.tidb WHERE VARIABLE_NAME='%s';\" to manually check the config. "+ - "if you ensure the value '%s' in backup cluster is as same as restore cluster, use --check-requirements=false to skip this check", - utils.TidbNewCollationEnabled, utils.TidbNewCollationEnabled, utils.TidbNewCollationEnabled) + "the value 'new_collations_enabled_on_first_bootstrap' not found in backupmeta. "+ + "you can use \"SELECT VARIABLE_VALUE FROM mysql.tidb WHERE VARIABLE_NAME='new_collations_enabled_on_first_bootstrap';\" to manually check the config. "+ + "if you ensure the value 'new_collations_enabled_on_first_bootstrap' in backup cluster is as same as restore cluster, use --check-requirements=false to skip this check", + ) } - log.Warn(fmt.Sprintf("the config '%s' is not in backupmeta", utils.TidbNewCollationEnabled)) + log.Warn(fmt.Sprintf("the config 'new_collations_enabled_on_first_bootstrap' is not in backupmeta")) return enabled, nil } if !strings.EqualFold(backupNewCollationEnable, newCollationEnable) { return enabled, errors.Annotatef(berrors.ErrUnknown, - "the config '%s' not match, upstream:%v, downstream: %v", - utils.TidbNewCollationEnabled, backupNewCollationEnable, newCollationEnable) + "the config 'new_collations_enabled_on_first_bootstrap' not match, upstream:%v, downstream: %v", + backupNewCollationEnable, newCollationEnable) } return enabled, nil ->>>>>>> 49484b19661 (restore: correct new collation when "--check-requirements=false" (#49579)) } type waitTiFlashBackoffer struct { From f89ced5f69885c9ffc26397d1a85b5a16609d5ea Mon Sep 17 00:00:00 2001 From: BornChanger <97348524+BornChanger@users.noreply.github.com> Date: Wed, 24 Jan 2024 21:17:16 +0800 Subject: [PATCH 3/3] Update br/pkg/restore/client.go --- br/pkg/restore/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/br/pkg/restore/client.go b/br/pkg/restore/client.go index 440f0c30dcd0b..58bf97b031df0 100644 --- a/br/pkg/restore/client.go +++ b/br/pkg/restore/client.go @@ -2906,7 +2906,7 @@ func CheckNewCollationEnable( "if you ensure the value 'new_collations_enabled_on_first_bootstrap' in backup cluster is as same as restore cluster, use --check-requirements=false to skip this check", ) } - log.Warn(fmt.Sprintf("the config 'new_collations_enabled_on_first_bootstrap' is not in backupmeta")) + log.Warn("the config 'new_collations_enabled_on_first_bootstrap' is not in backupmeta") return enabled, nil }