diff --git a/ddl/ddl_tiflash_api.go b/ddl/ddl_tiflash_api.go index ac200a9667655..199f11d366ed5 100644 --- a/ddl/ddl_tiflash_api.go +++ b/ddl/ddl_tiflash_api.go @@ -572,6 +572,9 @@ func (d *ddl) PollTiFlashRoutine() { if err != nil { logutil.BgLogger().Fatal("TiFlashManagement init failed", zap.Error(err)) } + + hasSetTiFlashGroup := false + nextSetTiFlashGroupTime := time.Now() for { select { case <-d.ctx.Done(): @@ -586,6 +589,18 @@ func (d *ddl) PollTiFlashRoutine() { failpoint.Inject("BeforePollTiFlashReplicaStatusLoop", func() { failpoint.Continue() }) + + if !hasSetTiFlashGroup && !time.Now().Before(nextSetTiFlashGroupTime) { + // We should set tiflash rule group a higher index than other placement groups to forbid override by them. + // Once `SetTiFlashGroupConfig` succeed, we do not need to invoke it again. If failed, we should retry it util success. + if err = infosync.SetTiFlashGroupConfig(d.ctx); err != nil { + logutil.BgLogger().Warn("SetTiFlashGroupConfig failed", zap.Error(err)) + nextSetTiFlashGroupTime = time.Now().Add(time.Minute) + } else { + hasSetTiFlashGroup = true + } + } + sctx, err := d.sessPool.get() if err == nil { if d.ownerManager.IsOwner() { diff --git a/ddl/ddl_tiflash_test.go b/ddl/ddl_tiflash_test.go index bd4efc96c3dac..32a08ec656e69 100644 --- a/ddl/ddl_tiflash_test.go +++ b/ddl/ddl_tiflash_test.go @@ -947,3 +947,21 @@ func TestTiFlashBatchUnsupported(t *testing.T) { require.Equal(t, "In total 2 tables: 1 succeed, 0 failed, 1 skipped", tk.Session().GetSessionVars().StmtCtx.GetMessage()) tk.MustGetErrCode("alter database information_schema set tiflash replica 1", 8200) } + +func TestTiFlashGroupIndexWhenStartup(t *testing.T) { + s, teardown := createTiFlashContext(t) + defer teardown() + _ = testkit.NewTestKit(t, s.store) + timeout := time.Now().Add(10 * time.Second) + errMsg := "time out" + for time.Now().Before(timeout) { + time.Sleep(100 * time.Millisecond) + if s.tiflash.GroupIndex != 0 { + errMsg = "invalid group index" + break + } + } + require.Equal(t, placement.RuleIndexTiFlash, s.tiflash.GroupIndex, errMsg) + require.Greater(t, s.tiflash.GroupIndex, placement.RuleIndexTable) + require.Greater(t, s.tiflash.GroupIndex, placement.RuleIndexPartition) +} diff --git a/ddl/partition.go b/ddl/partition.go index d024ef47b4b43..1a93cfdfb7f9b 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -239,10 +239,6 @@ func alterTablePartitionBundles(t *meta.Meta, tblInfo *model.TableInfo, addingDe p.Definitions = append(tblInfo.Partition.Definitions, addingDefinitions...) tblInfo.Partition = &p - if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 && tableHasPlacementSettings(tblInfo) { - return nil, errors.Trace(dbterror.ErrIncompatibleTiFlashAndPlacement) - } - // bundle for table should be recomputed because it includes some default configs for partitions tblBundle, err := placement.NewTableBundle(t, tblInfo) if err != nil { diff --git a/ddl/placement/common.go b/ddl/placement/common.go index 8421a6cbbc33e..cd02622dd0562 100644 --- a/ddl/placement/common.go +++ b/ddl/placement/common.go @@ -19,6 +19,8 @@ import ( ) const ( + // TiFlashRuleGroupID is the rule group id of tiflash + TiFlashRuleGroupID = "tiflash" // BundleIDPrefix is the bundle prefix of all rule bundles from TiDB_DDL statements. BundleIDPrefix = "TiDB_DDL_" // PDBundleID is the bundle name of pd, the default bundle for all regions. diff --git a/ddl/placement/rule.go b/ddl/placement/rule.go index dcce5653c05c2..2e91df421dc21 100644 --- a/ddl/placement/rule.go +++ b/ddl/placement/rule.go @@ -36,6 +36,13 @@ const ( Learner PeerRoleType = "learner" ) +// RuleGroupConfig defines basic config of rule group +type RuleGroupConfig struct { + ID string `json:"id"` + Index int `json:"index"` + Override bool `json:"override"` +} + // Rule is the core placement rule struct. Check https://github.com/tikv/pd/blob/master/server/schedule/placement/rule.go. type Rule struct { GroupID string `json:"group_id"` diff --git a/ddl/placement_policy.go b/ddl/placement_policy.go index e38797fb22a9e..eedb4bbf09dcf 100644 --- a/ddl/placement_policy.go +++ b/ddl/placement_policy.go @@ -417,19 +417,3 @@ func checkPlacementPolicyNotUsedByTable(tblInfo *model.TableInfo, policy *model. return nil } - -func tableHasPlacementSettings(tblInfo *model.TableInfo) bool { - if tblInfo.PlacementPolicyRef != nil { - return true - } - - if tblInfo.Partition != nil { - for _, def := range tblInfo.Partition.Definitions { - if def.PlacementPolicyRef != nil { - return true - } - } - } - - return false -} diff --git a/ddl/placement_sql_test.go b/ddl/placement_sql_test.go index 42ef1e29986e8..83fa4cc09249e 100644 --- a/ddl/placement_sql_test.go +++ b/ddl/placement_sql_test.go @@ -21,11 +21,11 @@ import ( "github.com/pingcap/failpoint" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" + "github.com/pingcap/tidb/domain/infosync" mysql "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/testkit" - "github.com/pingcap/tidb/util/dbterror" "github.com/stretchr/testify/require" ) @@ -583,8 +583,23 @@ func TestPlacementMode(t *testing.T) { } +func checkTiflashReplicaSet(t *testing.T, do *domain.Domain, db, tb string, cnt uint64) { + tbl, err := do.InfoSchema().TableByName(model.NewCIStr(db), model.NewCIStr(tb)) + require.NoError(t, err) + + tiflashReplica := tbl.Meta().TiFlashReplica + if cnt == 0 { + require.Nil(t, tiflashReplica) + return + } + + CheckPlacementRule(infosync.GetMockTiFlash(), *infosync.MakeNewRule(tbl.Meta().ID, 1, nil)) + require.NotNil(t, tiflashReplica) + require.Equal(t, cnt, tiflashReplica.Count) +} + func TestPlacementTiflashCheck(t *testing.T) { - store, clean := testkit.CreateMockStore(t) + store, do, clean := testkit.CreateMockStoreAndDomain(t) defer clean() tk := testkit.NewTestKit(t, store) require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/infoschema/mockTiFlashStoreCount", `return(true)`)) @@ -593,39 +608,65 @@ func TestPlacementTiflashCheck(t *testing.T) { require.NoError(t, err) }() + tiflash := infosync.NewMockTiFlash() + infosync.SetMockTiFlash(tiflash) + defer func() { + tiflash.Lock() + tiflash.StatusServer.Close() + tiflash.Unlock() + }() + tk.MustExec("use test") tk.MustExec("drop placement policy if exists p1") + tk.MustExec("drop placement policy if exists p2") 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 placement policy p2 primary_region='r2' regions='r1,r2'") + defer tk.MustExec("drop placement policy if exists p2") + 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") + checkTiflashReplicaSet(t, do, "test", "tp", 0) tk.MustExec("alter table tp set tiflash replica 1") - err := tk.ExecToErr("alter table tp placement policy p1") - require.True(t, dbterror.ErrIncompatibleTiFlashAndPlacement.Equal(err)) - err = tk.ExecToErr("alter table tp partition p0 placement policy p1") - require.True(t, dbterror.ErrIncompatibleTiFlashAndPlacement.Equal(err)) + tk.MustExec("alter table tp placement policy p1") + checkExistTableBundlesInPD(t, do, "test", "tp") + checkTiflashReplicaSet(t, do, "test", "tp", 1) 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" + + ") 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("alter table tp partition p0 placement policy p2") + checkExistTableBundlesInPD(t, do, "test", "tp") + checkTiflashReplicaSet(t, do, "test", "tp", 1) + 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) /*T![placement] PLACEMENT POLICY=`p2` */,\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") - require.True(t, dbterror.ErrIncompatibleTiFlashAndPlacement.Equal(err)) + checkTiflashReplicaSet(t, do, "test", "tp", 0) + + tk.MustExec("alter table tp set tiflash replica 1") + checkTiflashReplicaSet(t, do, "test", "tp", 1) + checkExistTableBundlesInPD(t, do, "test", "tp") tk.MustQuery("show create table tp").Check(testkit.Rows("" + "tp CREATE TABLE `tp` (\n" + " `id` int(11) DEFAULT NULL\n" + @@ -639,8 +680,11 @@ func TestPlacementTiflashCheck(t *testing.T) { 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") - require.True(t, dbterror.ErrIncompatibleTiFlashAndPlacement.Equal(err)) + checkTiflashReplicaSet(t, do, "test", "tp", 0) + + tk.MustExec("alter table tp set tiflash replica 1") + checkExistTableBundlesInPD(t, do, "test", "tp") + checkTiflashReplicaSet(t, do, "test", "tp", 1) tk.MustQuery("show create table tp").Check(testkit.Rows("" + "tp CREATE TABLE `tp` (\n" + " `id` int(11) DEFAULT NULL\n" + @@ -654,8 +698,11 @@ func TestPlacementTiflashCheck(t *testing.T) { PARTITION p0 VALUES LESS THAN (100), PARTITION p1 VALUES LESS THAN (1000) )`) - err = tk.ExecToErr("alter table tp set tiflash replica 1") - require.True(t, dbterror.ErrIncompatibleTiFlashAndPlacement.Equal(err)) + checkTiflashReplicaSet(t, do, "test", "tp", 0) + + tk.MustExec("alter table tp set tiflash replica 1") + checkExistTableBundlesInPD(t, do, "test", "tp") + checkTiflashReplicaSet(t, do, "test", "tp", 1) tk.MustQuery("show create table tp").Check(testkit.Rows("" + "tp CREATE TABLE `tp` (\n" + " `id` int(11) DEFAULT NULL\n" + @@ -669,8 +716,11 @@ func TestPlacementTiflashCheck(t *testing.T) { 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") - require.True(t, dbterror.ErrIncompatibleTiFlashAndPlacement.Equal(err)) + checkTiflashReplicaSet(t, do, "test", "tp", 0) + + tk.MustExec("alter table tp set tiflash replica 1") + checkExistTableBundlesInPD(t, do, "test", "tp") + checkTiflashReplicaSet(t, do, "test", "tp", 1) tk.MustQuery("show create table tp").Check(testkit.Rows("" + "tp CREATE TABLE `tp` (\n" + " `id` int(11) DEFAULT NULL\n" + diff --git a/ddl/table.go b/ddl/table.go index fc1f8769330d5..f84d140cf6d4a 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -1076,11 +1076,6 @@ func (w *worker) onSetTableFlashReplica(d *ddlCtx, t *meta.Meta, job *model.Job) return ver, errors.Trace(err) } - if replicaInfo.Count > 0 && tableHasPlacementSettings(tblInfo) { - job.State = model.JobStateCancelled - return ver, errors.Trace(dbterror.ErrIncompatibleTiFlashAndPlacement) - } - // Ban setting replica count for tables in system database. if tidb_util.IsMemOrSysDB(job.SchemaName) { return ver, errors.Trace(dbterror.ErrUnsupportedAlterReplicaForSysTable) @@ -1419,11 +1414,6 @@ func onAlterTablePartitionPlacement(d *ddlCtx, t *meta.Meta, job *model.Job) (ve return 0, err } - if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 { - job.State = model.JobStateCancelled - return 0, errors.Trace(dbterror.ErrIncompatibleTiFlashAndPlacement) - } - ptInfo := tblInfo.GetPartitionInfo() var partitionDef *model.PartitionDefinition definitions := ptInfo.Definitions @@ -1489,11 +1479,6 @@ func onAlterTablePlacement(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, return 0, err } - if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 { - job.State = model.JobStateCancelled - return 0, errors.Trace(dbterror.ErrIncompatibleTiFlashAndPlacement) - } - if _, err = checkPlacementPolicyRefValidAndCanNonValidJob(t, job, policyRefInfo); err != nil { return 0, errors.Trace(err) } diff --git a/domain/infosync/info.go b/domain/infosync/info.go index 07fbbb524f7c9..921d4669e6884 100644 --- a/domain/infosync/info.go +++ b/domain/infosync/info.go @@ -973,6 +973,16 @@ func GetLabelRules(ctx context.Context, ruleIDs []string) (map[string]*label.Rul return is.labelRuleManager.GetLabelRules(ctx, ruleIDs) } +// SetTiFlashGroupConfig is a helper function to set tiflash rule group config +func SetTiFlashGroupConfig(ctx context.Context) error { + is, err := getGlobalInfoSyncer() + if err != nil { + return errors.Trace(err) + } + logutil.BgLogger().Info("SetTiFlashGroupConfig") + return is.tiflashPlacementManager.SetTiFlashGroupConfig(ctx) +} + // SetTiFlashPlacementRule is a helper function to set placement rule. // It is discouraged to use SetTiFlashPlacementRule directly, // use `ConfigureTiFlashPDForTable`/`ConfigureTiFlashPDForPartitions` instead. diff --git a/domain/infosync/tiflash_manager.go b/domain/infosync/tiflash_manager.go index 48d526840ea40..62e154092bcc0 100644 --- a/domain/infosync/tiflash_manager.go +++ b/domain/infosync/tiflash_manager.go @@ -43,6 +43,8 @@ import ( // TiFlashPlacementManager manages placement settings for TiFlash. type TiFlashPlacementManager interface { + // SetTiFlashGroupConfig sets the group index of the tiflash placement rule + SetTiFlashGroupConfig(ctx context.Context) error // SetPlacementRule is a helper function to set placement rule. SetPlacementRule(ctx context.Context, rule placement.TiFlashRule) error // DeletePlacementRule is to delete placement rule for certain group. @@ -69,8 +71,63 @@ func (m *TiFlashPDPlacementManager) Close(ctx context.Context) { } +// SetTiFlashGroupConfig sets the tiflash's rule group config +func (m *TiFlashPDPlacementManager) SetTiFlashGroupConfig(ctx context.Context) error { + res, err := doRequest(ctx, + "GetRuleGroupConfig", + m.etcdCli.Endpoints(), + path.Join(pdapi.Config, "rule_group", placement.TiFlashRuleGroupID), + "GET", + nil, + ) + + if err != nil { + return errors.Trace(err) + } + + var groupConfig placement.RuleGroupConfig + shouldUpdate := res == nil + if res != nil { + if err = json.Unmarshal(res, &groupConfig); err != nil { + return errors.Trace(err) + } + + if groupConfig.Index != placement.RuleIndexTiFlash || groupConfig.Override { + shouldUpdate = true + } + } + + if shouldUpdate { + groupConfig.ID = placement.TiFlashRuleGroupID + groupConfig.Index = placement.RuleIndexTiFlash + groupConfig.Override = false + + body, err := json.Marshal(&groupConfig) + if err != nil { + return errors.Trace(err) + } + + _, err = doRequest(ctx, + "SetRuleGroupConfig", + m.etcdCli.Endpoints(), + path.Join(pdapi.Config, "rule_group"), + "POST", + bytes.NewBuffer(body), + ) + + if err != nil { + return errors.Trace(err) + } + } + return nil +} + // SetPlacementRule is a helper function to set placement rule. func (m *TiFlashPDPlacementManager) SetPlacementRule(ctx context.Context, rule placement.TiFlashRule) error { + if err := m.SetTiFlashGroupConfig(ctx); err != nil { + return err + } + if rule.Count == 0 { return m.DeletePlacementRule(ctx, rule.GroupID, rule.ID) } @@ -195,7 +252,7 @@ type mockTiFlashPlacementManager struct { func makeBaseRule() placement.TiFlashRule { return placement.TiFlashRule{ - GroupID: "tiflash", + GroupID: placement.TiFlashRuleGroupID, ID: "", Index: placement.RuleIndexTiFlash, Override: false, @@ -248,6 +305,7 @@ func (m *mockTiFlashTableInfo) String() string { // MockTiFlash mocks a TiFlash, with necessary Pd support. type MockTiFlash struct { sync.Mutex + GroupIndex int StatusAddr string StatusServer *httptest.Server SyncStatus map[int]mockTiFlashTableInfo @@ -315,6 +373,7 @@ func NewMockTiFlash() *MockTiFlash { func (tiflash *MockTiFlash) HandleSetPlacementRule(rule placement.TiFlashRule) error { tiflash.Lock() defer tiflash.Unlock() + tiflash.GroupIndex = placement.RuleIndexTiFlash if !tiflash.PdEnabled { logutil.BgLogger().Info("pd server is manually disabled, just quit") return nil @@ -525,6 +584,17 @@ func (tiflash *MockTiFlash) PdSwitch(enabled bool) { tiflash.PdEnabled = enabled } +// SetTiFlashGroupConfig sets the tiflash's rule group config +func (m *mockTiFlashPlacementManager) SetTiFlashGroupConfig(_ context.Context) error { + m.Lock() + defer m.Unlock() + if m.tiflash == nil { + return nil + } + m.tiflash.GroupIndex = placement.RuleIndexTiFlash + return nil +} + // SetPlacementRule is a helper function to set placement rule. func (m *mockTiFlashPlacementManager) SetPlacementRule(ctx context.Context, rule placement.TiFlashRule) error { m.Lock() diff --git a/util/dbterror/ddl_terror.go b/util/dbterror/ddl_terror.go index bcfe9b7a02730..293cc7da10ce2 100644 --- a/util/dbterror/ddl_terror.go +++ b/util/dbterror/ddl_terror.go @@ -365,8 +365,6 @@ var ( ErrDependentByFunctionalIndex = ClassDDL.NewStd(mysql.ErrDependentByFunctionalIndex) // ErrFunctionalIndexOnBlob when the expression of expression index returns blob or text. ErrFunctionalIndexOnBlob = ClassDDL.NewStd(mysql.ErrFunctionalIndexOnBlob) - // ErrIncompatibleTiFlashAndPlacement when placement and tiflash replica options are set at the same time - ErrIncompatibleTiFlashAndPlacement = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message("Placement and tiflash replica options cannot be set at the same time", nil)) // ErrAutoConvert when auto convert happens ErrAutoConvert = ClassDDL.NewStd(mysql.ErrAutoConvert)