config: improve scheduler config reload behavior (#2592)

server/cluster/coordinator_test.go

@@ -672,8 +672,11 @@ func (s *testCoordinatorSuite) TestPersistScheduler(c *C) {
 	_, err = schedule.CreateScheduler(schedulers.AdjacentRegionType, oc, storage, schedule.ConfigJSONDecoder([]byte("null")))
 	c.Assert(err, IsNil)
 	// suppose we add a new default enable scheduler
-	newOpt.AddSchedulerCfg(schedulers.AdjacentRegionType, []string{})
-	c.Assert(newOpt.GetSchedulers(), HasLen, 5)
+	config.DefaultSchedulers = append(config.DefaultSchedulers, config.SchedulerConfig{Type: "adjacent-region"})
+	defer func() {
+		config.DefaultSchedulers = config.DefaultSchedulers[:len(config.DefaultSchedulers)-1]
+	}()
+	c.Assert(newOpt.GetSchedulers(), HasLen, 4)
 	c.Assert(newOpt.Reload(storage), IsNil)
 	// only remains 3 items with independent config.
 	sches, _, err = storage.LoadAllScheduleConfig()

server/config/config.go

@@ -768,7 +768,7 @@ func (c *ScheduleConfig) adjust(meta *configMetaData) error {
 	}
 	adjustFloat64(&c.LowSpaceRatio, defaultLowSpaceRatio)
 	adjustFloat64(&c.HighSpaceRatio, defaultHighSpaceRatio)
-	adjustSchedulers(&c.Schedulers, defaultSchedulers)
+	adjustSchedulers(&c.Schedulers, DefaultSchedulers)
 
 	for k, b := range c.migrateConfigurationMap() {
 		v, err := c.parseDeprecatedFlag(meta, k, *b[0], *b[1])
@@ -895,7 +895,10 @@ type SchedulerConfig struct {
 	ArgsPayload string   `toml:"args-payload" json:"args-payload"`
 }
 
-var defaultSchedulers = SchedulerConfigs{
+// DefaultSchedulers are the schedulers be created by default.
+// If these schedulers are not in the persistent configuration, they
+// will be created automatically when reloading.
+var DefaultSchedulers = SchedulerConfigs{
 	{Type: "balance-region"},
 	{Type: "balance-leader"},
 	{Type: "hot-region"},
@@ -904,7 +907,7 @@ var defaultSchedulers = SchedulerConfigs{
 
 // IsDefaultScheduler checks whether the scheduler is enable by default.
 func IsDefaultScheduler(typ string) bool {
-	for _, c := range defaultSchedulers {
+	for _, c := range DefaultSchedulers {
 		if typ == c.Type {
 			return true
 		}

server/config/config_test.go

@@ -62,17 +62,20 @@ func (s *testConfigSuite) TestReloadConfig(c *C) {
 	opt.GetPDServerConfig().UseRegionStorage = true
 	c.Assert(opt.Persist(storage), IsNil)
 
-	// suppose we add a new default enable scheduler "adjacent-region"
-	defaultSchedulers := []string{"balance-region", "balance-leader", "hot-region", "label", "adjacent-region"}
+	// Add a new default enable scheduler "adjacent-region"
+	DefaultSchedulers = append(DefaultSchedulers, SchedulerConfig{Type: "adjacent-region"})
+	defer func() {
+		DefaultSchedulers = DefaultSchedulers[:len(DefaultSchedulers)-1]
+	}()
+
 	newOpt, err := newTestScheduleOption()
 	c.Assert(err, IsNil)
-	newOpt.AddSchedulerCfg("adjacent-region", []string{})
 	c.Assert(newOpt.Reload(storage), IsNil)
 	schedulers := newOpt.GetSchedulers()
 	c.Assert(schedulers, HasLen, 5)
 	c.Assert(newOpt.IsUseRegionStorage(), IsTrue)
 	for i, s := range schedulers {
-		c.Assert(s.Type, Equals, defaultSchedulers[i])
+		c.Assert(s.Type, Equals, DefaultSchedulers[i].Type)
 		c.Assert(s.Disable, IsFalse)
 	}
 	c.Assert(newOpt.GetMaxReplicas(), Equals, 5)

server/config/persist_options.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/pingcap/kvproto/pkg/metapb"
+	"github.com/pingcap/pd/v4/pkg/slice"
 	"github.com/pingcap/pd/v4/pkg/typeutil"
 	"github.com/pingcap/pd/v4/server/core"
 	"github.com/pingcap/pd/v4/server/kv"
@@ -481,19 +482,12 @@ func (o *PersistOptions) Persist(storage *core.Storage) error {
 
 // Reload reloads the configuration from the storage.
 func (o *PersistOptions) Reload(storage *core.Storage) error {
-	cfg := &Config{
-		Schedule:        *o.GetScheduleConfig().Clone(),
-		Replication:     *o.GetReplicationConfig().clone(),
-		PDServerCfg:     *o.GetPDServerConfig().Clone(),
-		ReplicationMode: *o.GetReplicationModeConfig().Clone(),
-		LabelProperty:   o.GetLabelPropertyConfig().Clone(),
-		ClusterVersion:  *o.GetClusterVersion(),
-	}
+	cfg := &Config{}
 	isExist, err := storage.LoadConfig(cfg)
 	if err != nil {
 		return err
 	}
-	o.adjustScheduleCfg(cfg)
+	o.adjustScheduleCfg(&cfg.Schedule)
 	if isExist {
 		o.schedule.Store(&cfg.Schedule)
 		o.replication.Store(&cfg.Replication)
@@ -505,33 +499,16 @@ func (o *PersistOptions) Reload(storage *core.Storage) error {
 	return nil
 }
 
-func (o *PersistOptions) adjustScheduleCfg(persistentCfg *Config) {
-	scheduleCfg := o.GetScheduleConfig().Clone()
-	for i, s := range scheduleCfg.Schedulers {
-		for _, ps := range persistentCfg.Schedule.Schedulers {
-			if s.Type == ps.Type && reflect.DeepEqual(s.Args, ps.Args) {
-				scheduleCfg.Schedulers[i].Disable = ps.Disable
-				break
-			}
-		}
-	}
-	restoredSchedulers := make([]SchedulerConfig, 0, len(persistentCfg.Schedule.Schedulers))
-	for _, ps := range persistentCfg.Schedule.Schedulers {
-		needRestore := true
-		for _, s := range scheduleCfg.Schedulers {
-			if s.Type == ps.Type && reflect.DeepEqual(s.Args, ps.Args) {
-				needRestore = false
-				break
-			}
-		}
-		if needRestore {
-			restoredSchedulers = append(restoredSchedulers, ps)
+func (o *PersistOptions) adjustScheduleCfg(scheduleCfg *ScheduleConfig) {
+	// In case we add new default schedulers.
+	for _, ps := range DefaultSchedulers {
+		if slice.NoneOf(scheduleCfg.Schedulers, func(i int) bool {
+			return scheduleCfg.Schedulers[i].Type == ps.Type
+		}) {
+			scheduleCfg.Schedulers = append(scheduleCfg.Schedulers, ps)
 		}
 	}
-	scheduleCfg.Schedulers = append(scheduleCfg.Schedulers, restoredSchedulers...)
-	persistentCfg.Schedule.Schedulers = scheduleCfg.Schedulers
-	persistentCfg.Schedule.MigrateDeprecatedFlags()
-	o.SetScheduleConfig(scheduleCfg)
+	scheduleCfg.MigrateDeprecatedFlags()
 }
 
 // CheckLabelProperty checks the label property.