Skip to content

server: change PD region label isolate level Histogram to Gauge #976

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

Merged
merged 7 commits into from
Mar 14, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions server/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,22 @@ type clusterInfo struct {
sync.RWMutex
*schedule.BasicCluster

id core.IDAllocator
kv *core.KV
meta *metapb.Cluster
activeRegions int
opt *scheduleOption
regionStats *regionStatistics
id core.IDAllocator
kv *core.KV
meta *metapb.Cluster
activeRegions int
opt *scheduleOption
regionStats *regionStatistics
labelLevelStats *labelLevelStatistics
}

func newClusterInfo(id core.IDAllocator, opt *scheduleOption, kv *core.KV) *clusterInfo {
return &clusterInfo{
BasicCluster: schedule.NewBasicCluster(),
id: id,
opt: opt,
kv: kv,
BasicCluster: schedule.NewBasicCluster(),
id: id,
opt: opt,
kv: kv,
labelLevelStats: newLabelLevelStatistics(),
}
}

Expand Down Expand Up @@ -470,6 +472,12 @@ func (c *clusterInfo) handleRegionHeartbeat(region *core.RegionInfo) error {
}
}
}
for _, item := range overlaps {
if c.regionStats != nil {
c.regionStats.clearDefunctRegion(item.GetId())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some way to call this function in RemoveRegion(), maybe other places will call the RemoveRegion() in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use less this method, otherwise, the logic is more confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misunderstood, but now the statistics not belong to RegionsInfo, it also needs to know some stores information.

}
c.labelLevelStats.clearDefunctRegion(item.GetId())
}

// Update related stores.
if origin != nil {
Expand All @@ -483,7 +491,7 @@ func (c *clusterInfo) handleRegionHeartbeat(region *core.RegionInfo) error {
}

if c.regionStats != nil {
c.regionStats.Observe(region, c.getRegionStores(region), c.GetLocationLabels())
c.regionStats.Observe(region, c.getRegionStores(region))
}

key := region.GetId()
Expand All @@ -504,13 +512,22 @@ func (c *clusterInfo) handleRegionHeartbeat(region *core.RegionInfo) error {
return nil
}

func (c *clusterInfo) updateRegionsLabelLevelStats(regions []*core.RegionInfo) {
c.Lock()
defer c.Unlock()
for _, region := range regions {
c.labelLevelStats.Observe(region, c.getRegionStores(region), c.GetLocationLabels())
}
}

func (c *clusterInfo) collectMetrics() {
if c.regionStats == nil {
return
}
c.RLock()
defer c.RUnlock()
c.regionStats.Collect()
c.labelLevelStats.Collect()
}

func (c *clusterInfo) GetRegionStatsByType(typ regionStatisticType) []*core.RegionInfo {
Expand Down
2 changes: 2 additions & 0 deletions server/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ func (c *coordinator) patrolRegions() {
break
}
}
// update label level isolation statistics.
c.cluster.updateRegionsLabelLevelStats(regions)
}
}

Expand Down
11 changes: 5 additions & 6 deletions server/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,13 @@ var (
Help: "Status of the regions.",
}, []string{"type"})

regionLabelLevelHistogram = prometheus.NewHistogram(
prometheus.HistogramOpts{
regionLabelLevelGauge = prometheus.NewGaugeVec(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to change a metrics' type? Can prometheus handle it right? /cc @overvenus

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gauge use pd_regions_label_level as metrics name, it's a base name in Histogram for three metrics with suffix "bucket","count" and "sum".They do not conflict./cc @overvenus

prometheus.GaugeOpts{
Namespace: "pd",
Subsystem: "regions",
Name: "label_level",
Help: "Bucketed histogram of the label level of the region.",
Buckets: prometheus.LinearBuckets(0, 1, 8),
})
Help: "Number of regions in the different label level.",
}, []string{"type"})

timeJumpBackCounter = prometheus.NewCounter(
prometheus.CounterOpts{
Expand Down Expand Up @@ -155,6 +154,6 @@ func init() {
prometheus.MustRegister(tsoCounter)
prometheus.MustRegister(storeStatusGauge)
prometheus.MustRegister(regionStatusGauge)
prometheus.MustRegister(regionLabelLevelHistogram)
prometheus.MustRegister(regionLabelLevelGauge)
prometheus.MustRegister(metadataGauge)
}
66 changes: 54 additions & 12 deletions server/region_statistics.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package server

import (
"fmt"

"github.com/pingcap/pd/server/core"
"github.com/pingcap/pd/server/namespace"
)
Expand Down Expand Up @@ -66,7 +68,7 @@ func (r *regionStatistics) deleteEntry(deleteIndex regionStatisticType, regionID
}
}

func (r *regionStatistics) Observe(region *core.RegionInfo, stores []*core.StoreInfo, labels []string) {
func (r *regionStatistics) Observe(region *core.RegionInfo, stores []*core.StoreInfo) {
// Region state.
regionID := region.GetId()
namespace := r.classifier.GetRegionNamespace(region)
Expand Down Expand Up @@ -104,11 +106,59 @@ func (r *regionStatistics) Observe(region *core.RegionInfo, stores []*core.Store
}
r.deleteEntry(deleteIndex, regionID)
r.index[regionID] = peerTypeIndex
if len(stores) == 0 {
return
}

func (r *regionStatistics) clearDefunctRegion(regionID uint64) {
if oldIndex, ok := r.index[regionID]; ok {
r.deleteEntry(oldIndex, regionID)
}
}

func (r *regionStatistics) Collect() {
regionStatusGauge.WithLabelValues("miss_peer_region_count").Set(float64(len(r.stats[missPeer])))
regionStatusGauge.WithLabelValues("extra_peer_region_count").Set(float64(len(r.stats[extraPeer])))
regionStatusGauge.WithLabelValues("down_peer_region_count").Set(float64(len(r.stats[downPeer])))
regionStatusGauge.WithLabelValues("pending_peer_region_count").Set(float64(len(r.stats[pendingPeer])))
regionStatusGauge.WithLabelValues("incorrect_namespace_region_count").Set(float64(len(r.stats[incorrectNamespace])))
}

type labelLevelStatistics struct {
regionLabelLevelStats map[uint64]int
labelLevelCounter map[int]int
}

func newLabelLevelStatistics() *labelLevelStatistics {
return &labelLevelStatistics{
regionLabelLevelStats: make(map[uint64]int),
labelLevelCounter: make(map[int]int),
}
}

func (l *labelLevelStatistics) Observe(region *core.RegionInfo, stores []*core.StoreInfo, labels []string) {
regionID := region.GetId()
regionLabelLevel := getRegionLabelIsolationLevel(stores, labels)
regionLabelLevelHistogram.Observe(float64(regionLabelLevel))
if level, ok := l.regionLabelLevelStats[regionID]; ok {
if level == regionLabelLevel {
return
}
l.labelLevelCounter[level]--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if region was merged, so there is no heartbeat anymore, namely Observe will not be called. Maybe we should add a function which can delete all relative stats of merged region.

}
l.regionLabelLevelStats[regionID] = regionLabelLevel
l.labelLevelCounter[regionLabelLevel]++
}

func (l *labelLevelStatistics) Collect() {
for level, count := range l.labelLevelCounter {
typ := fmt.Sprintf("level_%d", level)
regionStatusGauge.WithLabelValues(typ).Set(float64(count))
}
}

func (l *labelLevelStatistics) clearDefunctRegion(regionID uint64) {
if level, ok := l.regionLabelLevelStats[regionID]; ok {
l.labelLevelCounter[level]--
delete(l.regionLabelLevelStats, regionID)
}
}

func getRegionLabelIsolationLevel(stores []*core.StoreInfo, labels []string) int {
Expand Down Expand Up @@ -149,11 +199,3 @@ func notIsolatedStoresWithLabel(stores []*core.StoreInfo, label string) [][]*cor
}
return res
}

func (r *regionStatistics) Collect() {
regionStatusGauge.WithLabelValues("miss_peer_region_count").Set(float64(len(r.stats[missPeer])))
regionStatusGauge.WithLabelValues("extra_peer_region_count").Set(float64(len(r.stats[extraPeer])))
regionStatusGauge.WithLabelValues("down_peer_region_count").Set(float64(len(r.stats[downPeer])))
regionStatusGauge.WithLabelValues("pending_peer_region_count").Set(float64(len(r.stats[pendingPeer])))
regionStatusGauge.WithLabelValues("incorrect_namespace_region_count").Set(float64(len(r.stats[incorrectNamespace])))
}
17 changes: 13 additions & 4 deletions server/region_statistics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,18 @@ func (t *testRegionStatistcs) TestRegionStatistics(c *C) {
region1 := core.NewRegionInfo(r1, peers[0])
region2 := core.NewRegionInfo(r2, peers[0])
regionStats := newRegionStatistics(opt, mockClassifier{})
regionStats.Observe(region1, stores, nil)
regionStats.Observe(region1, stores)
c.Assert(len(regionStats.stats[extraPeer]), Equals, 1)
region1.DownPeers = downPeers
region1.PendingPeers = peers[0:1]
regionStats.Observe(region1, stores, nil)
regionStats.Observe(region1, stores)
c.Assert(len(regionStats.stats[extraPeer]), Equals, 1)
c.Assert(len(regionStats.stats[missPeer]), Equals, 0)
c.Assert(len(regionStats.stats[downPeer]), Equals, 1)
c.Assert(len(regionStats.stats[pendingPeer]), Equals, 1)
c.Assert(len(regionStats.stats[incorrectNamespace]), Equals, 1)
region2.DownPeers = downPeers[0:1]
regionStats.Observe(region2, stores[0:2], nil)
regionStats.Observe(region2, stores[0:2])
c.Assert(len(regionStats.stats[extraPeer]), Equals, 1)
c.Assert(len(regionStats.stats[missPeer]), Equals, 1)
c.Assert(len(regionStats.stats[downPeer]), Equals, 2)
Expand All @@ -95,6 +95,7 @@ func (t *testRegionStatistcs) TestRegionStatistics(c *C) {
}

func (t *testRegionStatistcs) TestRegionLabelIsolationLevel(c *C) {
labelLevelStats := newLabelLevelStatistics()
labelsSet := [][]map[string]string{
{
{"zone": "z1", "rack": "r1", "host": "h1"},
Expand Down Expand Up @@ -123,6 +124,8 @@ func (t *testRegionStatistcs) TestRegionLabelIsolationLevel(c *C) {
},
}
res := []int{2, 3, 1, 2, 0}
counter := []int{1, 1, 2, 1, 0}
regionID := 1
f := func(labels []map[string]string, res int) {
metaStores := []*metapb.Store{
{Id: 1, Address: "mock://tikv-1"},
Expand All @@ -137,14 +140,20 @@ func (t *testRegionStatistcs) TestRegionLabelIsolationLevel(c *C) {
}
stores = append(stores, s)
}
region := core.NewRegionInfo(&metapb.Region{Id: uint64(regionID)}, nil)
level := getRegionLabelIsolationLevel(stores, []string{"zone", "rack", "host"})
labelLevelStats.Observe(region, stores, []string{"zone", "rack", "host"})
c.Assert(level, Equals, res)
regionID++
}

for i, labels := range labelsSet {
f(labels, res[i])

}
for i, res := range counter {
c.Assert(labelLevelStats.labelLevelCounter[i], Equals, res)
}

level := getRegionLabelIsolationLevel(nil, []string{"zone", "rack", "host"})
c.Assert(level, Equals, 0)
level = getRegionLabelIsolationLevel(nil, nil)
Expand Down