From 5b5bc83b732f6724178a9508d8418a77e1184bb5 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 12 Mar 2021 21:54:55 +0800 Subject: [PATCH] statistics: fix the bug that the isolation level is wrong when the store lacks label (#3467) (#3474) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * cherry pick #3467 to release-4.0 Signed-off-by: ti-srebot * fix Signed-off-by: HunDunDM Co-authored-by: 混沌DM --- server/cluster/cluster.go | 2 +- server/statistics/region_collection.go | 56 ++++++++++++--------- server/statistics/region_collection_test.go | 23 +++++++-- 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go index 93a2abf4ce6..17e6967c5ae 100644 --- a/server/cluster/cluster.go +++ b/server/cluster/cluster.go @@ -635,7 +635,7 @@ func (c *RaftCluster) processRegionHeartbeat(region *core.RegionInfo) error { if c.regionStats != nil { c.regionStats.ClearDefunctRegion(item.GetID()) } - c.labelLevelStats.ClearDefunctRegion(item.GetID(), c.GetLocationLabels()) + c.labelLevelStats.ClearDefunctRegion(item.GetID()) } // Update related stores. diff --git a/server/statistics/region_collection.go b/server/statistics/region_collection.go index 4b94e612004..5f7dd62be55 100644 --- a/server/statistics/region_collection.go +++ b/server/statistics/region_collection.go @@ -189,10 +189,10 @@ func (l *LabelStatistics) Observe(region *core.RegionInfo, stores []*core.StoreI if label == regionIsolation { return } - l.counterDec(label) + l.labelCounter[label]-- } l.regionLabelStats[regionID] = regionIsolation - l.counterInc(regionIsolation) + l.labelCounter[regionIsolation]++ } // Collect collects the metrics of the label status. @@ -208,26 +208,10 @@ func (l *LabelStatistics) Reset() { } // ClearDefunctRegion is used to handle the overlap region. -func (l *LabelStatistics) ClearDefunctRegion(regionID uint64, labels []string) { +func (l *LabelStatistics) ClearDefunctRegion(regionID uint64) { if label, ok := l.regionLabelStats[regionID]; ok { - l.counterDec(label) - delete(l.regionLabelStats, regionID) - } -} - -func (l *LabelStatistics) counterInc(label string) { - if label == nonIsolation { - l.labelCounter[nonIsolation]++ - } else { - l.labelCounter[label]++ - } -} - -func (l *LabelStatistics) counterDec(label string) { - if label == nonIsolation { - l.labelCounter[nonIsolation]-- - } else { l.labelCounter[label]-- + delete(l.regionLabelStats, regionID) } } @@ -253,17 +237,39 @@ func getRegionLabelIsolation(stores []*core.StoreInfo, labels []string) string { } func notIsolatedStoresWithLabel(stores []*core.StoreInfo, label string) [][]*core.StoreInfo { - m := make(map[string][]*core.StoreInfo) + var emptyValueStores []*core.StoreInfo + valueStoresMap := make(map[string][]*core.StoreInfo) + for _, s := range stores { labelValue := s.GetLabelValue(label) if labelValue == "" { - continue + emptyValueStores = append(emptyValueStores, s) + } else { + valueStoresMap[labelValue] = append(valueStoresMap[labelValue], s) + } + } + + if len(valueStoresMap) == 0 { + // Usually it is because all TiKVs lack this label. + if len(emptyValueStores) > 1 { + return [][]*core.StoreInfo{emptyValueStores} } - m[labelValue] = append(m[labelValue], s) + return nil } + var res [][]*core.StoreInfo - for _, stores := range m { - if len(stores) > 1 { + if len(emptyValueStores) == 0 { + // No TiKV lacks this label. + for _, stores := range valueStoresMap { + if len(stores) > 1 { + res = append(res, stores) + } + } + } else { + // Usually it is because some TiKVs lack this label. + // The TiKVs in each label and the TiKVs without label form a group. + for _, stores := range valueStoresMap { + stores = append(stores, emptyValueStores...) res = append(res, stores) } } diff --git a/server/statistics/region_collection_test.go b/server/statistics/region_collection_test.go index 18b140de9f8..428a22a00b9 100644 --- a/server/statistics/region_collection_test.go +++ b/server/statistics/region_collection_test.go @@ -190,9 +190,21 @@ func (t *testRegionStatisticsSuite) TestRegionLabelIsolationLevel(c *C) { {"zone": "z1", "rack": "r2", "host": "h2"}, {"zone": "z1", "rack": "r2", "host": "h2"}, }, + { + // isolated by rack + {"rack": "r1", "host": "h1"}, + {"rack": "r2", "host": "h2"}, + {"rack": "r3", "host": "h3"}, + }, + { + // isolated by host + {"zone": "z1", "rack": "r1", "host": "h1"}, + {"zone": "z1", "rack": "r2", "host": "h2"}, + {"zone": "z1", "host": "h3"}, + }, } - res := []string{"rack", "host", "zone", "rack", "none"} - counter := map[string]int{"none": 1, "host": 1, "rack": 2, "zone": 1} + res := []string{"rack", "host", "zone", "rack", "none", "rack", "host"} + counter := map[string]int{"none": 1, "host": 2, "rack": 3, "zone": 1} regionID := 1 f := func(labels []map[string]string, res string, locationLabels []string) { metaStores := []*metapb.Store{ @@ -228,10 +240,13 @@ func (t *testRegionStatisticsSuite) TestRegionLabelIsolationLevel(c *C) { c.Assert(label, Equals, nonIsolation) label = getRegionLabelIsolation(nil, nil) c.Assert(label, Equals, nonIsolation) + store := core.NewStoreInfo(&metapb.Store{Id: 1, Address: "mock://tikv-1"}, core.SetStoreLabels([]*metapb.StoreLabel{{Key: "foo", Value: "bar"}})) + label = getRegionLabelIsolation([]*core.StoreInfo{store}, locationLabels) + c.Assert(label, Equals, "zone") regionID = 1 - res = []string{"rack", "none", "zone", "rack", "none"} - counter = map[string]int{"none": 2, "host": 0, "rack": 2, "zone": 1} + res = []string{"rack", "none", "zone", "rack", "none", "rack", "none"} + counter = map[string]int{"none": 3, "host": 0, "rack": 3, "zone": 1} locationLabels = []string{"zone", "rack"} for i, labels := range labelsSet {