Skip to content

Commit

Permalink
*: some picks to 3.0 (#1585)
Browse files Browse the repository at this point in the history
* config: warn undefined config item (#1577)

* config: warn undefined config item

Signed-off-by: nolouch <nolouch@gmail.com>

* *: add option to check label strictly (#1553)

* *: add option to check tikv label strictly

Signed-off-by: nolouch <nolouch@gmail.com>

* config: disable label check by default (#1568)

Signed-off-by: nolouch <nolouch@gmail.com>
  • Loading branch information
nolouch authored and disksing committed Jun 17, 2019
1 parent 772fb85 commit 9ccec24
Show file tree
Hide file tree
Showing 11 changed files with 187 additions and 11 deletions.
3 changes: 3 additions & 0 deletions conf/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ merge-schedule-limit = 8
tolerant-size-ratio = 0
# Enable two-way merge, set it to true may help improving merge speed.
#enable-two-way-merge = false
#tolerant-size-ratio = 5.0

# customized schedulers, the format is as below
# if empty, it will use balance-leader, balance-region, hot-region as default
Expand All @@ -85,6 +86,8 @@ max-replicas = 3
# For example, ["zone", "rack"] means that we should place replicas to
# different zones first, then to different racks if we don't have enough zones.
location-labels = []
# Strictly checks if the label of TiKV is matched with location labels.
#strictly-match-label = false

[label-property]
# Do not assign region leaders to stores that have these tags.
Expand Down
110 changes: 109 additions & 1 deletion server/api/label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@
package api

import (
"context"
"fmt"
"strings"

. "github.com/pingcap/check"
"github.com/pingcap/kvproto/pkg/metapb"
"github.com/pingcap/kvproto/pkg/pdpb"
"github.com/pingcap/pd/server"
)

var _ = Suite(&testLabelsStoreSuite{})
var _ = Suite(&testStrictlyLabelsStoreSuite{})

type testLabelsStoreSuite struct {
svr *server.Server
Expand Down Expand Up @@ -102,7 +106,7 @@ func (s *testLabelsStoreSuite) SetUpSuite(c *C) {
},
}

s.svr, s.cleanup = mustNewServer(c)
s.svr, s.cleanup = mustNewServer(c, func(cfg *server.Config) { cfg.Replication.StrictlyMatchLabel = false })
mustWaitLeader(c, []*server.Server{s.svr})

addr := s.svr.GetAddr()
Expand Down Expand Up @@ -170,3 +174,107 @@ func (s *testLabelsStoreSuite) TestStoresLabelFilter(c *C) {
_, err := newStoresLabelFilter("test", ".[test")
c.Assert(err, NotNil)
}

type testStrictlyLabelsStoreSuite struct {
svr *server.Server
cleanup cleanUpFunc
urlPrefix string
}

func (s *testStrictlyLabelsStoreSuite) SetUpSuite(c *C) {
s.svr, s.cleanup = mustNewServer(c, func(cfg *server.Config) {
cfg.Replication.LocationLabels = []string{"zone", "disk"}
cfg.Replication.StrictlyMatchLabel = true
})
mustWaitLeader(c, []*server.Server{s.svr})

addr := s.svr.GetAddr()
s.urlPrefix = fmt.Sprintf("%s%s/api/v1", addr, apiPrefix)

mustBootstrapCluster(c, s.svr)
}

func (s *testStrictlyLabelsStoreSuite) TestStoreMatch(c *C) {
cases := []struct {
store *metapb.Store
valid bool
expectError string
}{
{
store: &metapb.Store{
Id: 1,
Address: "tikv1",
State: metapb.StoreState_Up,
Labels: []*metapb.StoreLabel{
{
Key: "zone",
Value: "us-west-1",
},
{
Key: "disk",
Value: "ssd",
},
},
Version: "3.0.0",
},
valid: true,
},
{
store: &metapb.Store{
Id: 2,
Address: "tikv2",
State: metapb.StoreState_Up,
Labels: []*metapb.StoreLabel{},
Version: "3.0.0",
},
valid: false,
expectError: "label configuration is incorrect",
},
{
store: &metapb.Store{
Id: 2,
Address: "tikv2",
State: metapb.StoreState_Up,
Labels: []*metapb.StoreLabel{
{
Key: "zone",
Value: "cn-beijing-1",
},
{
Key: "disk",
Value: "ssd",
},
{
Key: "other",
Value: "unknown",
},
},
Version: "3.0.0",
},
valid: false,
expectError: "key matching the label was not found",
},
}

for _, t := range cases {
_, err := s.svr.PutStore(context.Background(), &pdpb.PutStoreRequest{
Header: &pdpb.RequestHeader{ClusterId: s.svr.ClusterID()},
Store: &metapb.Store{
Id: t.store.Id,
Address: fmt.Sprintf("tikv%d", t.store.Id),
State: t.store.State,
Labels: t.store.Labels,
Version: t.store.Version,
},
})
if t.valid {
c.Assert(err, IsNil)
} else {
c.Assert(strings.Contains(err.Error(), t.expectError), IsTrue)
}
}
}

func (s *testStrictlyLabelsStoreSuite) TearDownSuite(c *C) {
s.cleanup()
}
9 changes: 6 additions & 3 deletions server/api/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ func cleanServer(cfg *server.Config) {

type cleanUpFunc func()

func mustNewServer(c *C) (*server.Server, cleanUpFunc) {
_, svrs, cleanup := mustNewCluster(c, 1)
func mustNewServer(c *C, opts ...func(cfg *server.Config)) (*server.Server, cleanUpFunc) {
_, svrs, cleanup := mustNewCluster(c, 1, opts...)
return svrs[0], cleanup
}

var zapLogOnce sync.Once

func mustNewCluster(c *C, num int) ([]*server.Config, []*server.Server, cleanUpFunc) {
func mustNewCluster(c *C, num int, opts ...func(cfg *server.Config)) ([]*server.Config, []*server.Server, cleanUpFunc) {
svrs := make([]*server.Server, 0, num)
cfgs := server.NewTestMultiConfig(c, num)

Expand All @@ -89,6 +89,9 @@ func mustNewCluster(c *C, num int) ([]*server.Config, []*server.Server, cleanUpF
zapLogOnce.Do(func() {
log.ReplaceGlobals(cfg.GetZapLogger(), cfg.GetZapLogProperties())
})
for _, opt := range opts {
opt(cfg)
}
s, err := server.CreateServer(cfg, NewHandler)
c.Assert(err, IsNil)
err = s.Run(context.TODO())
Expand Down
19 changes: 19 additions & 0 deletions server/api/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"io/ioutil"
"net/http"
"net/url"
"strings"
"time"

. "github.com/pingcap/check"
Expand Down Expand Up @@ -146,11 +147,23 @@ func (s *testStoreSuite) TestStoreLabel(c *C) {
c.Assert(err, IsNil)
c.Assert(info.Store.Labels, HasLen, 0)

// Test merge.
// enable label match check.
labelCheck := map[string]string{"strictly-match-label": "true"}
lc, _ := json.Marshal(labelCheck)
err = postJSON(s.urlPrefix+"/config", lc)
c.Assert(err, IsNil)
// Test set.
labels := map[string]string{"zone": "cn", "host": "local"}
b, err := json.Marshal(labels)
c.Assert(err, IsNil)
err = postJSON(url+"/label", b)
c.Assert(strings.Contains(err.Error(), "key matching the label was not found"), IsTrue)
locationLabels := map[string]string{"location-labels": "zone,host"}
ll, _ := json.Marshal(locationLabels)
err = postJSON(s.urlPrefix+"/config", ll)
c.Assert(err, IsNil)
err = postJSON(url+"/label", b)
c.Assert(err, IsNil)

err = readJSONWithURL(url, &info)
Expand All @@ -161,6 +174,12 @@ func (s *testStoreSuite) TestStoreLabel(c *C) {
}

// Test merge.
// disable label match check.
labelCheck = map[string]string{"strictly-match-label": "false"}
lc, _ = json.Marshal(labelCheck)
err = postJSON(s.urlPrefix+"/config", lc)
c.Assert(err, IsNil)

labels = map[string]string{"zack": "zack1", "Host": "host1"}
b, err = json.Marshal(labels)
c.Assert(err, IsNil)
Expand Down
20 changes: 18 additions & 2 deletions server/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,11 +428,27 @@ func (c *RaftCluster) putStore(store *metapb.Store) error {
)
}
// Check location labels.
for _, k := range c.cachedCluster.GetLocationLabels() {
keysSet := make(map[string]struct{})
for _, k := range cluster.GetLocationLabels() {
keysSet[k] = struct{}{}
if v := s.GetLabelValue(k); len(v) == 0 {
log.Warn("missing location label",
log.Warn("label configuration is incorrect",
zap.Stringer("store", s.GetMeta()),
zap.String("label-key", k))
if cluster.GetStrictlyMatchLabel() {
return errors.Errorf("label configuration is incorrect, need to specify the key: %s ", k)
}
}
}
for _, label := range s.GetLabels() {
key := label.GetKey()
if _, ok := keysSet[key]; !ok {
log.Warn("not found the key match with the store label",
zap.Stringer("store", s.GetMeta()),
zap.String("label-key", key))
if cluster.GetStrictlyMatchLabel() {
return errors.Errorf("key matching the label was not found in the PD, store label key: %s ", key)
}
}
}
return cluster.putStore(s)
Expand Down
4 changes: 4 additions & 0 deletions server/cluster_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,10 @@ func (c *clusterInfo) GetLocationLabels() []string {
return c.opt.GetLocationLabels()
}

func (c *clusterInfo) GetStrictlyMatchLabel() bool {
return c.opt.rep.GetStrictlyMatchLabel()
}

func (c *clusterInfo) GetHotRegionCacheHitsThreshold() int {
return c.opt.GetHotRegionCacheHitsThreshold()
}
Expand Down
15 changes: 11 additions & 4 deletions server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ const (

defaultLeaderPriorityCheckInterval = time.Minute

defaultUseRegionStorage = true
defaultUseRegionStorage = true
defaultStrictlyMatchLabel = false
)

func adjustString(v *string, defValue string) {
Expand Down Expand Up @@ -349,7 +350,7 @@ func (m *configMetaData) CheckUndecoded() error {
func (c *Config) Adjust(meta *toml.MetaData) error {
configMetaData := newConfigMetadata(meta)
if err := configMetaData.CheckUndecoded(); err != nil {
return err
c.WarningMsgs = append(c.WarningMsgs, err.Error())
}

if c.Name == "" {
Expand Down Expand Up @@ -683,14 +684,17 @@ type ReplicationConfig struct {
// For example, ["zone", "rack"] means that we should place replicas to
// different zones first, then to different racks if we don't have enough zones.
LocationLabels typeutil.StringSlice `toml:"location-labels,omitempty" json:"location-labels"`
// StrictlyMatchLabel strictly checks if the label of TiKV is matched with LocaltionLabels.
StrictlyMatchLabel bool `toml:"strictly-match-label,omitempty" json:"strictly-match-label,string"`
}

func (c *ReplicationConfig) clone() *ReplicationConfig {
locationLabels := make(typeutil.StringSlice, len(c.LocationLabels))
copy(locationLabels, c.LocationLabels)
return &ReplicationConfig{
MaxReplicas: c.MaxReplicas,
LocationLabels: locationLabels,
MaxReplicas: c.MaxReplicas,
LocationLabels: locationLabels,
StrictlyMatchLabel: c.StrictlyMatchLabel,
}
}

Expand All @@ -706,6 +710,9 @@ func (c *ReplicationConfig) validate() error {

func (c *ReplicationConfig) adjust(meta *configMetaData) error {
adjustUint64(&c.MaxReplicas, defaultMaxReplicas)
if !meta.IsDefined("strictly-match-label") {
c.StrictlyMatchLabel = defaultStrictlyMatchLabel
}
return c.validate()
}

Expand Down
4 changes: 3 additions & 1 deletion server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"fmt"
"os"
"path"
"strings"
"time"

"github.com/BurntSushi/toml"
Expand Down Expand Up @@ -133,7 +134,8 @@ type = "random-merge"
meta, err = toml.Decode(cfgData, &cfg)
c.Assert(err, IsNil)
err = cfg.Adjust(&meta)
c.Assert(err, NotNil)
c.Assert(err, IsNil)
c.Assert(strings.Contains(cfg.WarningMsgs[0], "Config contains undefined item"), IsTrue)

// Check misspelled schedulers name
cfgData = `
Expand Down
5 changes: 5 additions & 0 deletions server/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,11 @@ func (r *Replication) GetLocationLabels() []string {
return r.load().LocationLabels
}

// GetStrictlyMatchLabel returns whether check label strict.
func (r *Replication) GetStrictlyMatchLabel() bool {
return r.load().StrictlyMatchLabel
}

// namespaceOption is a wrapper to access the configuration safely.
type namespaceOption struct {
namespaceCfg atomic.Value
Expand Down
8 changes: 8 additions & 0 deletions server/schedule/mockcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ const (
defaultLowSpaceRatio = 0.8
defaultHighSpaceRatio = 0.6
defaultHotRegionCacheHitsThreshold = 3
defaultStrictlyMatchLabel = true
)

// MockSchedulerOptions is a mock of SchedulerOptions
Expand All @@ -559,6 +560,7 @@ type MockSchedulerOptions struct {
MaxStoreDownTime time.Duration
MaxReplicas int
LocationLabels []string
StrictlyMatchLabel bool
HotRegionCacheHitsThreshold int
TolerantSizeRatio float64
LowSpaceRatio float64
Expand Down Expand Up @@ -589,6 +591,7 @@ func NewMockSchedulerOptions() *MockSchedulerOptions {
mso.EnableTwoWayMerge = defaultTwoWayMerge
mso.MaxStoreDownTime = defaultMaxStoreDownTime
mso.MaxReplicas = defaultMaxReplicas
mso.StrictlyMatchLabel = defaultStrictlyMatchLabel
mso.HotRegionCacheHitsThreshold = defaultHotRegionCacheHitsThreshold
mso.MaxPendingPeerCount = defaultMaxPendingPeerCount
mso.TolerantSizeRatio = defaultTolerantSizeRatio
Expand Down Expand Up @@ -672,6 +675,11 @@ func (mso *MockSchedulerOptions) GetLocationLabels() []string {
return mso.LocationLabels
}

// GetStrictlyMatchLabel mock method
func (mso *MockSchedulerOptions) GetStrictlyMatchLabel() bool {
return mso.StrictlyMatchLabel
}

// GetHotRegionCacheHitsThreshold mock method
func (mso *MockSchedulerOptions) GetHotRegionCacheHitsThreshold() int {
return mso.HotRegionCacheHitsThreshold
Expand Down
1 change: 1 addition & 0 deletions server/schedule/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type Options interface {

GetMaxReplicas() int
GetLocationLabels() []string
GetStrictlyMatchLabel() bool

GetHotRegionCacheHitsThreshold() int
GetTolerantSizeRatio() float64
Expand Down

0 comments on commit 9ccec24

Please sign in to comment.