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

close #36453
pingcap · Jul 23, 2022 · 28a8ffc · 28a8ffc
1 parent 435dc58
commit 28a8ffc
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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"