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>
tikv · Nov 22, 2018 · 13a50b0 · 13a50b0
1 parent 5f47da2
commit 13a50b0
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 17 deletions.
diff --git a/conf/config.toml b/conf/config.toml
@@ -57,8 +57,8 @@ interval = "15s"
 address = ""
 
 [schedule]
-max-merge-region-size = 0
-max-merge-region-keys = 0
+max-merge-region-size = 20
+max-merge-region-keys = 200000
 split-merge-interval = "1h"
 max-snapshot-count = 3
 max-pending-peer-count = 16

diff --git a/server/config.go b/server/config.go
@@ -287,8 +287,37 @@ func (c *Config) validate() error {
 	return nil
 }
 
+// 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))
 
@@ -338,19 +367,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
@@ -489,19 +517,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)
@@ -583,7 +629,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()
 }

diff --git a/server/config_test.go b/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))
+}