From b9d9f19bd1f15cc7d28d572b3d2433c811a91b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E8=B6=85?= Date: Mon, 20 Dec 2021 16:47:46 +0800 Subject: [PATCH] *: forbid set tiflash replica count for a placement table (#30844) close pingcap/tidb#30741 --- ddl/error.go | 2 + ddl/placement_policy.go | 16 ++++++ ddl/placement_sql_test.go | 105 ++++++++++++++++++++++++++++++++++++++ ddl/table.go | 12 +++++ 4 files changed, 135 insertions(+) diff --git a/ddl/error.go b/ddl/error.go index f95b5baf0e4a9..6819920860e39 100644 --- 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 index 3ccba4ef346f6..80faeead391a5 100644 --- 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 index 62d6f36db1c4a..7dd419bac54fc 100644 --- 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 index cd2b818450c57..83f7ad0b0e58a 100644 --- 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) }