*: forbid set tiflash replica count for a placement table (#30844)

close #30741
pingcap · Dec 20, 2021 · b9d9f19 · b9d9f19
1 parent c44630e
commit b9d9f19
Show file tree

Hide file tree

Showing 4 changed files with 135 additions and 0 deletions.
diff --git a/ddl/error.go b/ddl/error.go
@@ -310,4 +310,6 @@ var (
 	errDependentByFunctionalIndex = dbterror.ClassDDL.NewStd(mysql.ErrDependentByFunctionalIndex)
 	// errFunctionalIndexOnBlob when the expression of expression index returns blob or text.
 	errFunctionalIndexOnBlob = dbterror.ClassDDL.NewStd(mysql.ErrFunctionalIndexOnBlob)
+	// ErrIncompatibleTiFlashAndPlacement when placement and tiflash replica options are set at the same time
+	ErrIncompatibleTiFlashAndPlacement = dbterror.ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message("Placement and tiflash replica options cannot be set at the same time", nil))
 )
diff --git a/ddl/placement_policy.go b/ddl/placement_policy.go
@@ -381,3 +381,19 @@ func checkPlacementPolicyNotUsedByTable(tblInfo *model.TableInfo, policy *model.
 
 	return nil
 }
+
+func tableHasPlacementSettings(tblInfo *model.TableInfo) bool {
+	if tblInfo.DirectPlacementOpts != nil || tblInfo.PlacementPolicyRef != nil {
+		return true
+	}
+
+	if tblInfo.Partition != nil {
+		for _, def := range tblInfo.Partition.Definitions {
+			if def.DirectPlacementOpts != nil || def.PlacementPolicyRef != nil {
+				return true
+			}
+		}
+	}
+
+	return false
+}
diff --git a/ddl/placement_sql_test.go b/ddl/placement_sql_test.go
@@ -21,6 +21,7 @@ import (
 
 	. "github.com/pingcap/check"
 	"github.com/pingcap/failpoint"
+	"github.com/pingcap/tidb/ddl"
 	"github.com/pingcap/tidb/ddl/placement"
 	mysql "github.com/pingcap/tidb/errno"
 	"github.com/pingcap/tidb/parser/model"
@@ -472,3 +473,107 @@ func (s *testDBSuite6) TestEnablePlacementCheck(c *C) {
 	tk.MustGetErrCode("create table m (c int) partition by range (c) (partition p1 values less than (200) followers=2);", mysql.ErrUnsupportedDDLOperation)
 	tk.MustGetErrCode("alter table t partition p1 placement policy=\"placement_x\";", mysql.ErrUnsupportedDDLOperation)
 }
+
+func (s *testDBSuite6) TestPlacementTiflashCheck(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	se, err := session.CreateSession4Test(s.store)
+	c.Assert(err, IsNil)
+	_, err = se.Execute(context.Background(), "set @@global.tidb_enable_alter_placement=1")
+	c.Assert(err, IsNil)
+
+	c.Assert(failpoint.Enable("github.com/pingcap/tidb/infoschema/mockTiFlashStoreCount", `return(true)`), IsNil)
+	defer func() {
+		err := failpoint.Disable("github.com/pingcap/tidb/infoschema/mockTiFlashStoreCount")
+		c.Assert(err, IsNil)
+	}()
+
+	tk.MustExec("use test")
+	tk.MustExec("drop placement policy if exists p1")
+	tk.MustExec("drop table if exists tp")
+
+	tk.MustExec("create placement policy p1 primary_region='r1' regions='r1'")
+	defer tk.MustExec("drop placement policy if exists p1")
+
+	tk.MustExec(`CREATE TABLE tp (id INT) PARTITION BY RANGE (id) (
+	   PARTITION p0 VALUES LESS THAN (100),
+	   PARTITION p1 VALUES LESS THAN (1000)
+	)`)
+	defer tk.MustExec("drop table if exists tp")
+	tk.MustExec("alter table tp set tiflash replica 1")
+
+	err = tk.ExecToErr("alter table tp placement policy p1")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	err = tk.ExecToErr("alter table tp primary_region='r2' regions='r2'")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	err = tk.ExecToErr("alter table tp partition p0 placement policy p1")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	err = tk.ExecToErr("alter table tp partition p0 primary_region='r2' regions='r2'")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	tk.MustQuery("show create table tp").Check(testkit.Rows("" +
+		"tp CREATE TABLE `tp` (\n" +
+		"  `id` int(11) DEFAULT NULL\n" +
+		") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
+		"PARTITION BY RANGE (`id`)\n" +
+		"(PARTITION `p0` VALUES LESS THAN (100),\n" +
+		" PARTITION `p1` VALUES LESS THAN (1000))"))
+
+	tk.MustExec("drop table tp")
+	tk.MustExec(`CREATE TABLE tp (id INT) placement policy p1 PARTITION BY RANGE (id) (
+	   PARTITION p0 VALUES LESS THAN (100),
+	   PARTITION p1 VALUES LESS THAN (1000)
+	)`)
+	err = tk.ExecToErr("alter table tp set tiflash replica 1")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	tk.MustQuery("show create table tp").Check(testkit.Rows("" +
+		"tp CREATE TABLE `tp` (\n" +
+		"  `id` int(11) DEFAULT NULL\n" +
+		") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PLACEMENT POLICY=`p1` */\n" +
+		"PARTITION BY RANGE (`id`)\n" +
+		"(PARTITION `p0` VALUES LESS THAN (100),\n" +
+		" PARTITION `p1` VALUES LESS THAN (1000))"))
+
+	tk.MustExec("drop table tp")
+	tk.MustExec(`CREATE TABLE tp (id INT) PARTITION BY RANGE (id) (
+        PARTITION p0 VALUES LESS THAN (100) placement policy p1 ,
+        PARTITION p1 VALUES LESS THAN (1000)
+	)`)
+	err = tk.ExecToErr("alter table tp set tiflash replica 1")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	tk.MustQuery("show create table tp").Check(testkit.Rows("" +
+		"tp CREATE TABLE `tp` (\n" +
+		"  `id` int(11) DEFAULT NULL\n" +
+		") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
+		"PARTITION BY RANGE (`id`)\n" +
+		"(PARTITION `p0` VALUES LESS THAN (100) /*T![placement] PLACEMENT POLICY=`p1` */,\n" +
+		" PARTITION `p1` VALUES LESS THAN (1000))"))
+
+	tk.MustExec("drop table tp")
+	tk.MustExec(`CREATE TABLE tp (id INT) primary_region='r2' regions='r2' PARTITION BY RANGE (id) (
+	   PARTITION p0 VALUES LESS THAN (100),
+	   PARTITION p1 VALUES LESS THAN (1000)
+	)`)
+	err = tk.ExecToErr("alter table tp set tiflash replica 1")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	tk.MustQuery("show create table tp").Check(testkit.Rows("" +
+		"tp CREATE TABLE `tp` (\n" +
+		"  `id` int(11) DEFAULT NULL\n" +
+		") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /*T![placement] PRIMARY_REGION=\"r2\" REGIONS=\"r2\" */\n" +
+		"PARTITION BY RANGE (`id`)\n" +
+		"(PARTITION `p0` VALUES LESS THAN (100),\n" +
+		" PARTITION `p1` VALUES LESS THAN (1000))"))
+
+	tk.MustExec("drop table tp")
+	tk.MustExec(`CREATE TABLE tp (id INT) PARTITION BY RANGE (id) (
+        PARTITION p0 VALUES LESS THAN (100)  primary_region='r3' regions='r3',
+        PARTITION p1 VALUES LESS THAN (1000)
+	)`)
+	err = tk.ExecToErr("alter table tp set tiflash replica 1")
+	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
+	tk.MustQuery("show create table tp").Check(testkit.Rows("" +
+		"tp CREATE TABLE `tp` (\n" +
+		"  `id` int(11) DEFAULT NULL\n" +
+		") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
+		"PARTITION BY RANGE (`id`)\n" +
+		"(PARTITION `p0` VALUES LESS THAN (100) /*T![placement] PRIMARY_REGION=\"r3\" REGIONS=\"r3\" */,\n" +
+		" PARTITION `p1` VALUES LESS THAN (1000))"))
+}
diff --git a/ddl/table.go b/ddl/table.go
@@ -954,6 +954,10 @@ func (w *worker) onSetTableFlashReplica(t *meta.Meta, job *model.Job) (ver int64
 		return ver, errors.Trace(err)
 	}
 
+	if replicaInfo.Count > 0 && tableHasPlacementSettings(tblInfo) {
+		return ver, errors.Trace(ErrIncompatibleTiFlashAndPlacement)
+	}
+
 	// Ban setting replica count for tables in system database.
 	if tidb_util.IsMemOrSysDB(job.SchemaName) {
 		return ver, errors.Trace(errUnsupportedAlterReplicaForSysTable)
@@ -1274,6 +1278,10 @@ func onAlterTablePartitionPlacement(t *meta.Meta, job *model.Job) (ver int64, er
 		return 0, err
 	}
 
+	if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 {
+		return 0, errors.Trace(ErrIncompatibleTiFlashAndPlacement)
+	}
+
 	ptInfo := tblInfo.GetPartitionInfo()
 	var partitionDef *model.PartitionDefinition
 	definitions := ptInfo.Definitions
@@ -1341,6 +1349,10 @@ func onAlterTablePlacement(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64,
 		return 0, err
 	}
 
+	if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 {
+		return 0, errors.Trace(ErrIncompatibleTiFlashAndPlacement)
+	}
+
 	if _, err = checkPlacementPolicyRefValidAndCanNonValidJob(t, job, policyRefInfo); err != nil {
 		return 0, errors.Trace(err)
 	}