config: fix configs cannot set to zero values (#1334)

* config: fix configs cannot set to zero values

Signed-off-by: disksing <i@disksing.com>

* update default config to keep behaivour consistent

Signed-off-by: disksing <i@disksing.com>

server/config.go

@@ -258,7 +258,7 @@ func (c *Config) Parse(arguments []string) error {
 		return errors.Errorf("'%s' is an invalid flag", c.FlagSet.Arg(0))
 	}
 
-	err = c.adjust(meta)
+	err = c.Adjust(meta)
 	return err
 }
 
@@ -285,7 +285,37 @@ func (c *Config) validate() error {
 	return nil
 }
 
-func (c *Config) adjust(meta *toml.MetaData) error {
+// Utility to test if a configuration is defined.
+type configMetaData struct {
+	meta *toml.MetaData
+	path []string
+}
+
+func newConfigMetadata(meta *toml.MetaData) *configMetaData {
+	return &configMetaData{meta: meta}
+}
+
+func (m *configMetaData) IsDefined(key string) bool {
+	if m.meta == nil {
+		return false
+	}
+	keys := append([]string(nil), m.path...)
+	keys = append(keys, key)
+	return m.meta.IsDefined(keys...)
+}
+
+func (m *configMetaData) Child(path ...string) *configMetaData {
+	newPath := append([]string(nil), m.path...)
+	newPath = append(newPath, path...)
+	return &configMetaData{
+		meta: m.meta,
+		path: newPath,
+	}
+}
+
+// Adjust is used to adjust the PD configurations.
+func (c *Config) Adjust(meta *toml.MetaData) error {
+	configMetaData := newConfigMetadata(meta)
 	adjustString(&c.Name, defaultName)
 	adjustString(&c.DataDir, fmt.Sprintf("default.%s", c.Name))
 
@@ -335,19 +365,18 @@ func (c *Config) adjust(meta *toml.MetaData) error {
 
 	adjustString(&c.Metric.PushJob, c.Name)
 
-	if err := c.Schedule.adjust(); err != nil {
+	if err := c.Schedule.adjust(configMetaData.Child("schedule")); err != nil {
 		return err
 	}
-	if err := c.Replication.adjust(); err != nil {
+	if err := c.Replication.adjust(configMetaData.Child("replication")); err != nil {
 		return err
 	}
 
 	adjustDuration(&c.heartbeatStreamBindInterval, defaultHeartbeatStreamRebindInterval)
 
 	adjustDuration(&c.leaderPriorityCheckInterval, defaultLeaderPriorityCheckInterval)
 
-	// enable PreVote by default
-	if meta == nil || !meta.IsDefined("enable-prevote") {
+	if !configMetaData.IsDefined("enable-prevote") {
 		c.PreVote = true
 	}
 	return nil
@@ -486,19 +515,37 @@ const (
 	defaultHighSpaceRatio       = 0.6
 )
 
-func (c *ScheduleConfig) adjust() error {
-	adjustUint64(&c.MaxSnapshotCount, defaultMaxSnapshotCount)
-	adjustUint64(&c.MaxPendingPeerCount, defaultMaxPendingPeerCount)
-	adjustUint64(&c.MaxMergeRegionSize, defaultMaxMergeRegionSize)
-	adjustUint64(&c.MaxMergeRegionKeys, defaultMaxMergeRegionKeys)
+func (c *ScheduleConfig) adjust(meta *configMetaData) error {
+	if !meta.IsDefined("max-snapshot-count") {
+		adjustUint64(&c.MaxSnapshotCount, defaultMaxSnapshotCount)
+	}
+	if !meta.IsDefined("max-pending-peer-count") {
+		adjustUint64(&c.MaxPendingPeerCount, defaultMaxPendingPeerCount)
+	}
+	if !meta.IsDefined("max-merge-region-size") {
+		adjustUint64(&c.MaxMergeRegionSize, defaultMaxMergeRegionSize)
+	}
+	if !meta.IsDefined("max-merge-region-keys") {
+		adjustUint64(&c.MaxMergeRegionKeys, defaultMaxMergeRegionKeys)
+	}
 	adjustDuration(&c.SplitMergeInterval, defaultSplitMergeInterval)
 	adjustDuration(&c.PatrolRegionInterval, defaultPatrolRegionInterval)
 	adjustDuration(&c.MaxStoreDownTime, defaultMaxStoreDownTime)
-	adjustUint64(&c.LeaderScheduleLimit, defaultLeaderScheduleLimit)
-	adjustUint64(&c.RegionScheduleLimit, defaultRegionScheduleLimit)
-	adjustUint64(&c.ReplicaScheduleLimit, defaultReplicaScheduleLimit)
-	adjustUint64(&c.MergeScheduleLimit, defaultMergeScheduleLimit)
-	adjustFloat64(&c.TolerantSizeRatio, defaultTolerantSizeRatio)
+	if !meta.IsDefined("leader-schedule-limit") {
+		adjustUint64(&c.LeaderScheduleLimit, defaultLeaderScheduleLimit)
+	}
+	if !meta.IsDefined("region-schedule-limit") {
+		adjustUint64(&c.RegionScheduleLimit, defaultRegionScheduleLimit)
+	}
+	if !meta.IsDefined("replica-schedule-limit") {
+		adjustUint64(&c.ReplicaScheduleLimit, defaultReplicaScheduleLimit)
+	}
+	if !meta.IsDefined("merge-schedule-limit") {
+		adjustUint64(&c.MergeScheduleLimit, defaultMergeScheduleLimit)
+	}
+	if !meta.IsDefined("tolerant-size-ratio") {
+		adjustFloat64(&c.TolerantSizeRatio, defaultTolerantSizeRatio)
+	}
 	adjustFloat64(&c.LowSpaceRatio, defaultLowSpaceRatio)
 	adjustFloat64(&c.HighSpaceRatio, defaultHighSpaceRatio)
 	adjustSchedulers(&c.Schedulers, defaultSchedulers)
@@ -580,7 +627,7 @@ func (c *ReplicationConfig) validate() error {
 	return nil
 }
 
-func (c *ReplicationConfig) adjust() error {
+func (c *ReplicationConfig) adjust(meta *configMetaData) error {
 	adjustUint64(&c.MaxReplicas, defaultMaxReplicas)
 	return c.validate()
 }

server/config_test.go

@@ -16,6 +16,8 @@ package server
 import (
 	"path"
 
+	"github.com/BurntSushi/toml"
+
 	. "github.com/pingcap/check"
 	"github.com/pingcap/pd/server/core"
 )
@@ -81,3 +83,29 @@ func (s *testConfigSuite) TestValidation(c *C) {
 	cfg.Schedule.TolerantSizeRatio = -0.6
 	c.Assert(cfg.Schedule.validate(), NotNil)
 }
+
+func (s *testConfigSuite) TestAdjust(c *C) {
+	cfgData := `
+name = ""
+lease = 0
+
+[schedule]
+max-merge-region-size = 0
+leader-schedule-limit = 0
+`
+	cfg := NewConfig()
+	meta, err := toml.Decode(cfgData, &cfg)
+	c.Assert(err, IsNil)
+	err = cfg.Adjust(&meta)
+	c.Assert(err, IsNil)
+
+	// When invalid, use default values.
+	c.Assert(cfg.Name, Equals, defaultName)
+	c.Assert(cfg.LeaderLease, Equals, defaultLeaderLease)
+	// When defined, use values from config file.
+	c.Assert(cfg.Schedule.MaxMergeRegionSize, Equals, uint64(0))
+	c.Assert(cfg.Schedule.LeaderScheduleLimit, Equals, uint64(0))
+	// When undefined, use default values.
+	c.Assert(cfg.PreVote, IsTrue)
+	c.Assert(cfg.Schedule.MaxMergeRegionKeys, Equals, uint64(defaultMaxMergeRegionKeys))
+}

server/testutil.go

@@ -79,7 +79,7 @@ func NewTestSingleConfig() *Config {
 	cfg.ElectionInterval = typeutil.NewDuration(3000 * time.Millisecond)
 	cfg.leaderPriorityCheckInterval = typeutil.NewDuration(100 * time.Millisecond)
 
-	cfg.adjust(nil)
+	cfg.Adjust(nil)
 
 	if schedule.Simulating {
 		cfg.Schedule.RegionScheduleLimit = 128