From 28a8ffcf67f9b1979920d03c29a54842314aa921 Mon Sep 17 00:00:00 2001 From: 3pointer Date: Sat, 23 Jul 2022 17:47:09 +0800 Subject: [PATCH] br: fix compatibility issue with concurrent ddl (#36474) close pingcap/tidb#36453 --- br/pkg/conn/conn.go | 9 ++++++ br/pkg/version/version.go | 13 +++++++++ br/pkg/version/version_test.go | 50 ++++++++++++++++++++++++++++++++++ br/tests/br_rawkv/run.sh | 4 +-- 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/br/pkg/conn/conn.go b/br/pkg/conn/conn.go index cd96e8f68a992..c5996c3530b87 100755 --- a/br/pkg/conn/conn.go +++ b/br/pkg/conn/conn.go @@ -241,6 +241,15 @@ func NewMgr( if err != nil { return nil, errors.Trace(err) } + // we must check tidb(tikv version) any time after concurrent ddl feature implemented in v6.2. + // when tidb < 6.2 we need set EnableConcurrentDDL false to make ddl works. + // we will keep this check until 7.0, which allow the breaking changes. + // NOTE: must call it after domain created! + // FIXME: remove this check in v7.0 + err = version.CheckClusterVersion(ctx, controller.GetPDClient(), version.CheckVersionForDDL) + if err != nil { + return nil, errors.Annotate(err, "unable to check cluster version for ddl") + } } mgr := &Mgr{ diff --git a/br/pkg/version/version.go b/br/pkg/version/version.go index 2a818a6ee4f85..692d19f8c4cf9 100644 --- a/br/pkg/version/version.go +++ b/br/pkg/version/version.go @@ -18,6 +18,7 @@ import ( "github.com/pingcap/tidb/br/pkg/logutil" "github.com/pingcap/tidb/br/pkg/utils" "github.com/pingcap/tidb/br/pkg/version/build" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/util/engine" pd "github.com/tikv/pd/client" "go.uber.org/zap" @@ -151,6 +152,18 @@ func CheckVersionForBRPiTR(s *metapb.Store, tikvVersion *semver.Version) error { return nil } +// 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. + requireVersion := semver.New("6.2.0-alpha") + if tikvVersion.Compare(*requireVersion) < 0 { + log.Info("detected the old version of tidb cluster. set enable concurrent ddl to false") + variable.EnableConcurrentDDL.Store(false) + return nil + } + return nil +} + // CheckVersionForBR checks whether version of the cluster and BR itself is compatible. func CheckVersionForBR(s *metapb.Store, tikvVersion *semver.Version) error { BRVersion, err := semver.NewVersion(removeVAndHash(build.ReleaseVersion)) diff --git a/br/pkg/version/version_test.go b/br/pkg/version/version_test.go index 008e8a10d1ccd..9527836fa500b 100644 --- a/br/pkg/version/version_test.go +++ b/br/pkg/version/version_test.go @@ -13,6 +13,7 @@ import ( "github.com/coreos/go-semver/semver" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/br/pkg/version/build" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/stretchr/testify/require" pd "github.com/tikv/pd/client" ) @@ -261,6 +262,55 @@ func TestCheckClusterVersion(t *testing.T) { require.Error(t, err) } + { + mock.getAllStores = func() []*metapb.Store { + return []*metapb.Store{{Version: "v6.4.0"}} + } + originVal := variable.EnableConcurrentDDL.Load() + err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) + require.NoError(t, err) + require.Equal(t, originVal, variable.EnableConcurrentDDL.Load()) + } + + { + mock.getAllStores = func() []*metapb.Store { + return []*metapb.Store{{Version: "v6.2.0"}} + } + originVal := variable.EnableConcurrentDDL.Load() + err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) + require.NoError(t, err) + require.Equal(t, originVal, variable.EnableConcurrentDDL.Load()) + } + + { + mock.getAllStores = func() []*metapb.Store { + return []*metapb.Store{{Version: "v6.2.0-alpha"}} + } + originVal := variable.EnableConcurrentDDL.Load() + err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) + require.NoError(t, err) + require.Equal(t, originVal, variable.EnableConcurrentDDL.Load()) + } + + { + mock.getAllStores = func() []*metapb.Store { + return []*metapb.Store{{Version: "v6.1.0"}} + } + variable.EnableConcurrentDDL.Store(true) + err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) + require.NoError(t, err) + require.False(t, variable.EnableConcurrentDDL.Load()) + } + + { + mock.getAllStores = func() []*metapb.Store { + return []*metapb.Store{{Version: "v5.4.0"}} + } + variable.EnableConcurrentDDL.Store(true) + err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) + require.NoError(t, err) + require.False(t, variable.EnableConcurrentDDL.Load()) + } } func TestCompareVersion(t *testing.T) { diff --git a/br/tests/br_rawkv/run.sh b/br/tests/br_rawkv/run.sh index b32cca0f8e41f..b8748d3e9fc1a 100755 --- a/br/tests/br_rawkv/run.sh +++ b/br/tests/br_rawkv/run.sh @@ -53,7 +53,7 @@ test_full_rawkv() { checksum_full=$(checksum $check_range_start $check_range_end) # backup current state of key-values # raw backup is not working with range [nil, nil]. TODO: fix it. - run_br --pd $PD_ADDR backup raw -s "local://$BACKUP_FULL" --crypter.method "aes128-ctr" --crypter.key "0123456789abcdef0123456789abcdef" --start $check_range_start --format hex + run_br --pd $PD_ADDR backup raw -s "local://$BACKUP_FULL" --crypter.method "aes128-ctr" --crypter.key "0123456789abcdef0123456789abcdef" --start $check_range_start --end $check_range_end --format hex clean $check_range_start $check_range_end # Ensure the data is deleted @@ -63,7 +63,7 @@ test_full_rawkv() { fail_and_exit fi - run_br --pd $PD_ADDR restore raw -s "local://$BACKUP_FULL" --crypter.method "aes128-ctr" --crypter.key "0123456789abcdef0123456789abcdef" --start $check_range_start --format hex + run_br --pd $PD_ADDR restore raw -s "local://$BACKUP_FULL" --crypter.method "aes128-ctr" --crypter.key "0123456789abcdef0123456789abcdef" --start $check_range_start --end $check_range_end --format hex checksum_new=$(checksum $check_range_start $check_range_end) if [ "$checksum_new" != "$checksum_full" ];then echo "failed to restore"