Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: add a check to reject changing some config items during runtime #14830

Merged
merged 11 commits into from
Feb 20, 2020
49 changes: 23 additions & 26 deletions config/config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,22 +168,33 @@ func (ch *pdConfHandler) register() {
return
}

newConf, err := decodeConfig(conf)
ch.updateConfig(conf, version)
ch.registered = true
wshwsh12 marked this conversation as resolved.
Show resolved Hide resolved
logutil.Logger(context.Background()).Info("PDConfHandler register successfully")
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
}

func (ch *pdConfHandler) updateConfig(newConfContent string, newVersion *configpb.Version) {
newConf, err := decodeConfig(newConfContent)
if err != nil {
logutil.Logger(context.Background()).Warn("decode config error when registering", zap.Error(err))
logutil.Logger(context.Background()).Warn("decode config error", zap.Error(err))
return
} else if err := newConf.Valid(); err != nil {
logutil.Logger(context.Background()).Warn("invalid remote config when registering", zap.Error(err))
logutil.Logger(context.Background()).Warn("invalid remote config", zap.Error(err))
return
}

ch.registered = true
ch.reloadFunc(ch.curConf.Load().(*Config), newConf)
ch.curConf.Store(newConf)
ch.version = version
logutil.Logger(context.Background()).Info("PDConfHandler register config successfully",
zap.String("version", version.String()),
zap.Any("new_config", newConf))
oldConf := ch.curConf.Load().(*Config)
mergedConf, err := CloneConf(oldConf)
if err != nil {
logutil.Logger(context.Background()).Warn("clone config error", zap.Error(err))
return
}
as, rs := MergeConfigItems(mergedConf, newConf)
ch.reloadFunc(oldConf, mergedConf)
ch.curConf.Store(mergedConf)
ch.version = newVersion
logutil.Logger(context.Background()).Info("PDConfHandler updates config successfully",
zap.String("new_version", newVersion.String()),
zap.Any("accepted_conf_items", as), zap.Any("rejected_conf_items", rs))
}

func (ch *pdConfHandler) run() {
Expand Down Expand Up @@ -220,22 +231,8 @@ func (ch *pdConfHandler) run() {
zap.Int("code", int(status.Code)), zap.String("message", status.Message))
continue
}
newConf, err := decodeConfig(newConfContent)
if err != nil {
logutil.Logger(context.Background()).Error("PDConfHandler decode config error", zap.Error(err))
continue
}
if err := newConf.Valid(); err != nil {
logutil.Logger(context.Background()).Error("PDConfHandler invalid config", zap.Error(err))
continue
}

ch.reloadFunc(ch.curConf.Load().(*Config), newConf)
ch.curConf.Store(newConf)
logutil.Logger(context.Background()).Info("PDConfHandler update config successfully",
zap.String("fromVersion", ch.version.String()), zap.String("toVersion", version.String()),
zap.Any("new_config", newConf))
ch.version = version
ch.updateConfig(newConfContent, version)
case <-ch.exit:
return
}
Expand Down
9 changes: 5 additions & 4 deletions config/config_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,21 @@ func (s *testConfigSuite) TestPDConfHandler(c *C) {
wg := sync.WaitGroup{}
wg.Add(1)
mockReloadFunc := func(oldConf, newConf *Config) {
c.Assert(oldConf.Log.Level, Equals, "info")
c.Assert(newConf.Log.Level, Equals, "debug")
c.Assert(oldConf.Performance.MaxMemory, Equals, uint64(233))
c.Assert(newConf.Performance.MaxMemory, Equals, uint64(123))
wg.Done()
}
conf.Performance.MaxMemory = 233
ch, err = newPDConfHandler(&conf, mockReloadFunc, newMockPDConfigClient)
c.Assert(err, IsNil)
ch.interval = time.Second
newConf := conf
newConf.Log.Level = "debug"
newConf.Performance.MaxMemory = 123
newContent, _ := encodeConfig(&newConf)
mockPDConfigClient0.confContent.Store(newContent)
ch.Start()
wg.Wait()
c.Assert(ch.GetConfig().Log.Level, Equals, "debug")
c.Assert(ch.GetConfig().Performance.MaxMemory, Equals, uint64(123))
ch.Close()
}

Expand Down
85 changes: 85 additions & 0 deletions config/config_util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright 2020 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package config

import (
"encoding/json"
"reflect"
)

// CloneConf ...
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
func CloneConf(conf *Config) (*Config, error) {
content, err := json.Marshal(conf)
if err != nil {
return nil, err
}
var clonedConf Config
if err := json.Unmarshal(content, &clonedConf); err != nil {
return nil, err
}
return &clonedConf, nil
}

var (
// dynamicConfigItems contains all config items that can be changed during runtime.
dynamicConfigItems = []string{
"Performance.MaxProcs", "Performance.MaxMemory",
"Performance.CrossJoin", "Performance.FeedbackProbability",
"Performance.QueryFeedbackLimit", "Performance.PseudoEstimateRatio",
"OOMAction", "MemQuotaQuery", "TiKVClient.StoreLimit",
}
)

// MergeConfigItems is used to overwrite some config items that can't be changed during runtime.
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
func MergeConfigItems(dstConf, newConf *Config) (acceptedItems, rejectedItems []string) {
return mergeConfigItemsWithAllowMap(dstConf, newConf, dynamicConfigItems)
}

func mergeConfigItemsWithAllowMap(dstConf, newConf *Config, dynamicConfigItems []string) (acceptedItems, rejectedItems []string) {
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
dynamicConfigItemsMap := make(map[string]struct{}, len(dynamicConfigItems))
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
for _, c := range dynamicConfigItems {
dynamicConfigItemsMap[c] = struct{}{}
}
return mergeConfigItems(reflect.ValueOf(dstConf), reflect.ValueOf(newConf), "", dynamicConfigItemsMap)
}

func mergeConfigItems(dstConf, newConf reflect.Value, fieldPath string, dynamicConfigItemsMap map[string]struct{}) (acceptedItems, rejectedItems []string) {
t := dstConf.Type()
if t.Kind() == reflect.Ptr {
t = t.Elem()
dstConf = dstConf.Elem()
newConf = newConf.Elem()
}
if t.Kind() != reflect.Struct {
if reflect.DeepEqual(dstConf.Interface(), newConf.Interface()) {
return
}
if _, ok := dynamicConfigItemsMap[fieldPath]; ok {
dstConf.Set(newConf)
return []string{fieldPath}, nil
}
return nil, []string{fieldPath}
}

for i := 0; i < t.NumField(); i++ {
p := t.Field(i).Name
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
if fieldPath != "" {
p = fieldPath + "." + p
}
as, rs := mergeConfigItems(dstConf.Field(i), newConf.Field(i), p, dynamicConfigItemsMap)
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
acceptedItems = append(acceptedItems, as...)
rejectedItems = append(rejectedItems, rs...)
}
return
}
98 changes: 98 additions & 0 deletions config/config_util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Copyright 2017 PingCAP, Inc.
wshwsh12 marked this conversation as resolved.
Show resolved Hide resolved
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package config

import (
"fmt"
"reflect"

. "github.com/pingcap/check"
)

func (s *testConfigSuite) TestCloneConf(c *C) {
c1, err := CloneConf(&defaultConf)
c.Assert(err, IsNil)
c2, err := CloneConf(c1)
c.Assert(err, IsNil)
c.Assert(reflect.DeepEqual(c1, c2), IsTrue)

c1.Store = "abc"
c1.Port = 2333
c1.Log.EnableSlowLog++
c1.RepairTableList = append(c1.RepairTableList, "abc")
c.Assert(c1.Store, Not(Equals), c2.Store)
c.Assert(c1.Port, Not(Equals), c2.Port)
c.Assert(c1.Log.EnableSlowLog, Not(Equals), c2.Log.EnableSlowLog)
c.Assert(fmt.Sprintf("%v", c1.RepairTableList), Not(Equals), fmt.Sprintf("%v", c2.RepairTableList))
}

func (s *testConfigSuite) TestMergeConfigItems(c *C) {
oriConf, _ := CloneConf(&defaultConf)
oldConf, _ := CloneConf(oriConf)
newConf, _ := CloneConf(oldConf)
allowed := []string{
"Performance.MaxProcs", "Performance.MaxMemory",
"Performance.CrossJoin", "Performance.FeedbackProbability",
"Performance.QueryFeedbackLimit", "Performance.PseudoEstimateRatio",
"OOMAction", "MemQuotaQuery", "TiKVClient.StoreLimit",
}
allowedMap := make(map[string]struct{})
for _, a := range allowed {
allowedMap[a] = struct{}{}
}

// allowed
newConf.Performance.MaxProcs = 123
newConf.Performance.MaxMemory = 123
newConf.Performance.CrossJoin = false
newConf.Performance.FeedbackProbability = 123
newConf.Performance.QueryFeedbackLimit = 123
newConf.Performance.PseudoEstimateRatio = 123
newConf.OOMAction = "panic"
newConf.MemQuotaQuery = 123
newConf.TiKVClient.StoreLimit = 123

// rejected
newConf.Store = "tiflash"
newConf.Port = 2333
newConf.AdvertiseAddress = "1.2.3.4"
newConf.Log.SlowThreshold = 2345

as, rs := mergeConfigItemsWithAllowMap(oldConf, newConf, allowed)
c.Assert(len(as), Equals, 9)
c.Assert(len(rs), Equals, 4)
for _, a := range as {
_, ok := allowedMap[a]
c.Assert(ok, IsTrue)
}
for _, a := range rs {
_, ok := allowedMap[a]
c.Assert(ok, IsFalse)
}

c.Assert(oldConf.Performance.MaxProcs, Equals, newConf.Performance.MaxProcs)
c.Assert(oldConf.Performance.MaxMemory, Equals, newConf.Performance.MaxMemory)
c.Assert(oldConf.Performance.CrossJoin, Equals, newConf.Performance.CrossJoin)
c.Assert(oldConf.Performance.FeedbackProbability, Equals, newConf.Performance.FeedbackProbability)
c.Assert(oldConf.Performance.QueryFeedbackLimit, Equals, newConf.Performance.QueryFeedbackLimit)
c.Assert(oldConf.Performance.PseudoEstimateRatio, Equals, newConf.Performance.PseudoEstimateRatio)
c.Assert(oldConf.OOMAction, Equals, newConf.OOMAction)
c.Assert(oldConf.MemQuotaQuery, Equals, newConf.MemQuotaQuery)
c.Assert(oldConf.TiKVClient.StoreLimit, Equals, newConf.TiKVClient.StoreLimit)

c.Assert(oldConf.Store, Equals, oriConf.Store)
c.Assert(oldConf.Port, Equals, oriConf.Port)
c.Assert(oldConf.AdvertiseAddress, Equals, oriConf.AdvertiseAddress)
c.Assert(oldConf.Log.SlowThreshold, Equals, oriConf.Log.SlowThreshold)
}