From 97e97e918a106a9f5c427f9e7e412e3586cb9e57 Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Thu, 6 Dec 2018 20:44:47 +0800 Subject: [PATCH 1/5] check undefined config --- server/config.go | 19 +++++++++++++++++++ server/config_test.go | 14 ++++++++++++++ server/coordinator.go | 2 +- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/server/config.go b/server/config.go index 93d4f372dd0..623683a868c 100644 --- a/server/config.go +++ b/server/config.go @@ -315,9 +315,28 @@ func (m *configMetaData) Child(path ...string) *configMetaData { } } +func (m *configMetaData) CheckUndecoded() error { + if m.meta == nil { + return nil + } + undecoded := m.meta.Undecoded() + if len(undecoded) == 0 { + return nil + } + errInfo := "Config contains undefined item: " + for _, key := range undecoded { + errInfo += key.String() + ", " + } + return errors.New(errInfo) +} + // Adjust is used to adjust the PD configurations. func (c *Config) Adjust(meta *toml.MetaData) error { configMetaData := newConfigMetadata(meta) + if err := configMetaData.CheckUndecoded(); err != nil { + return err + } + adjustString(&c.Name, defaultName) adjustString(&c.DataDir, fmt.Sprintf("default.%s", c.Name)) diff --git a/server/config_test.go b/server/config_test.go index d347aeee7fc..c06d2cba2cd 100644 --- a/server/config_test.go +++ b/server/config_test.go @@ -110,4 +110,18 @@ leader-schedule-limit = 0 // When undefined, use default values. c.Assert(cfg.PreVote, IsTrue) c.Assert(cfg.Schedule.MaxMergeRegionKeys, Equals, uint64(defaultMaxMergeRegionKeys)) + + cfgData = ` +lalala = "" +name = "" +lease = 0 + +[schedule] +type = "random-merge" +` + cfg = NewConfig() + meta, err = toml.Decode(cfgData, &cfg) + c.Assert(err, IsNil) + err = cfg.Adjust(&meta) + c.Assert(err, NotNil) } diff --git a/server/coordinator.go b/server/coordinator.go index 396163a9335..d9a82ebb8b7 100644 --- a/server/coordinator.go +++ b/server/coordinator.go @@ -199,7 +199,7 @@ func (c *coordinator) run() { } s, err := schedule.CreateScheduler(schedulerCfg.Type, c.opController, schedulerCfg.Args...) if err != nil { - log.Errorf("can not create scheduler %s: %v", schedulerCfg.Type, err) + panic(fmt.Sprintf("can not create scheduler %s: %v", schedulerCfg.Type, err)) } else { log.Infof("create scheduler %s", s.GetName()) if err = c.addScheduler(s, schedulerCfg.Args...); err != nil { From aa0b8608efb40482d3e49fcfea988ac4d036c3e3 Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Thu, 6 Dec 2018 21:58:12 +0800 Subject: [PATCH 2/5] improve --- server/config.go | 2 +- server/coordinator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/config.go b/server/config.go index 623683a868c..c70998e4008 100644 --- a/server/config.go +++ b/server/config.go @@ -327,7 +327,7 @@ func (m *configMetaData) CheckUndecoded() error { for _, key := range undecoded { errInfo += key.String() + ", " } - return errors.New(errInfo) + return errors.New(errInfo[:len(errInfo)-2]) } // Adjust is used to adjust the PD configurations. diff --git a/server/coordinator.go b/server/coordinator.go index d9a82ebb8b7..6b6995f3883 100644 --- a/server/coordinator.go +++ b/server/coordinator.go @@ -199,7 +199,7 @@ func (c *coordinator) run() { } s, err := schedule.CreateScheduler(schedulerCfg.Type, c.opController, schedulerCfg.Args...) if err != nil { - panic(fmt.Sprintf("can not create scheduler %s: %v", schedulerCfg.Type, err)) + log.Fatalf("can not create scheduler %s: %v", schedulerCfg.Type, err) } else { log.Infof("create scheduler %s", s.GetName()) if err = c.addScheduler(s, schedulerCfg.Args...); err != nil { From 39fa07dd25b05b966f179f065b8f8b9016a87759 Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Thu, 6 Dec 2018 22:09:43 +0800 Subject: [PATCH 3/5] fix ci --- server/coordinator.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/coordinator.go b/server/coordinator.go index 6b6995f3883..b7d4c824b53 100644 --- a/server/coordinator.go +++ b/server/coordinator.go @@ -200,11 +200,10 @@ func (c *coordinator) run() { s, err := schedule.CreateScheduler(schedulerCfg.Type, c.opController, schedulerCfg.Args...) if err != nil { log.Fatalf("can not create scheduler %s: %v", schedulerCfg.Type, err) - } else { - log.Infof("create scheduler %s", s.GetName()) - if err = c.addScheduler(s, schedulerCfg.Args...); err != nil { - log.Errorf("can not add scheduler %s: %v", s.GetName(), err) - } + } + log.Infof("create scheduler %s", s.GetName()) + if err = c.addScheduler(s, schedulerCfg.Args...); err != nil { + log.Errorf("can not add scheduler %s: %v", s.GetName(), err) } // only record valid scheduler config From 46ec7e372c62953c12096ea4bf9fde15a829a56c Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Mon, 10 Dec 2018 11:13:26 +0800 Subject: [PATCH 4/5] address comment --- server/config_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/server/config_test.go b/server/config_test.go index c06d2cba2cd..0b48a613651 100644 --- a/server/config_test.go +++ b/server/config_test.go @@ -111,13 +111,26 @@ leader-schedule-limit = 0 c.Assert(cfg.PreVote, IsTrue) c.Assert(cfg.Schedule.MaxMergeRegionKeys, Equals, uint64(defaultMaxMergeRegionKeys)) + // Check undefined config fields cfgData = ` -lalala = "" +type = "pd" name = "" lease = 0 [schedule] type = "random-merge" +` + cfg = NewConfig() + meta, err = toml.Decode(cfgData, &cfg) + c.Assert(err, IsNil) + err = cfg.Adjust(&meta) + c.Assert(err, NotNil) + + // Check wrong scheduler name + cfgData = ` +[[schedule.schedulers]] +type = "random-merge-scheduler" +args = [""] ` cfg = NewConfig() meta, err = toml.Decode(cfgData, &cfg) From 9189aefecde0598882b6af95b209722b253fe538 Mon Sep 17 00:00:00 2001 From: Connor1996 Date: Mon, 10 Dec 2018 14:18:01 +0800 Subject: [PATCH 5/5] fix --- server/config_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/server/config_test.go b/server/config_test.go index 0b48a613651..297a4e3da53 100644 --- a/server/config_test.go +++ b/server/config_test.go @@ -119,18 +119,6 @@ lease = 0 [schedule] type = "random-merge" -` - cfg = NewConfig() - meta, err = toml.Decode(cfgData, &cfg) - c.Assert(err, IsNil) - err = cfg.Adjust(&meta) - c.Assert(err, NotNil) - - // Check wrong scheduler name - cfgData = ` -[[schedule.schedulers]] -type = "random-merge-scheduler" -args = [""] ` cfg = NewConfig() meta, err = toml.Decode(cfgData, &cfg)