From f8116b70f38edfa13c11cfd4437222ca9e355f3f Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Wed, 13 Feb 2019 14:59:11 -0500 Subject: [PATCH 1/3] healthcheck: Cache regions in tablet_stats_cache Signed-off-by: Leo Xuzhang Lin --- go/vt/discovery/tablet_stats_cache.go | 36 +++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/go/vt/discovery/tablet_stats_cache.go b/go/vt/discovery/tablet_stats_cache.go index 7fa1809e5ce..f3bbe453ea2 100644 --- a/go/vt/discovery/tablet_stats_cache.go +++ b/go/vt/discovery/tablet_stats_cache.go @@ -56,6 +56,8 @@ type TabletStatsCache struct { entries map[string]map[string]map[topodatapb.TabletType]*tabletStatsCacheEntry // tsm is a helper to broadcast aggregate stats. tsm srvtopo.TargetStatsMultiplexer + // cellRegions is a cache of cell regions + cellRegions map[string]string } // tabletStatsCacheEntry is the per keyspace/shard/tabletType @@ -134,6 +136,7 @@ func newTabletStatsCache(hc HealthCheck, ts *topo.Server, cell string, setListen aggregatesChan: make(chan []*srvtopo.TargetStatsEntry, 100), entries: make(map[string]map[string]map[topodatapb.TabletType]*tabletStatsCacheEntry), tsm: srvtopo.NewTargetStatsMultiplexer(), + cellRegions: make(map[string]string), } if setListener { @@ -193,13 +196,36 @@ func (tc *TabletStatsCache) getOrCreateEntry(target *querypb.Target) *tabletStat return e } -func (tc *TabletStatsCache) getRegionByCell(cell string) string { - return topo.GetRegionByCell(context.Background(), tc.ts, cell) +func (tc *TabletStatsCache) getRegionByCell(cell string) (string, bool) { + tc.mu.RLock() + defer tc.mu.RUnlock() + if region, ok := tc.cellRegions[cell]; ok { + return region, true + } + return "", false +} + +func (tc *TabletStatsCache) getOrCreateRegionByCell(cell string) string { + // Fast path + if region, ok := tc.getRegionByCell(cell); ok { + return region + } + + // Slow path + tc.mu.Lock() + defer tc.mu.Unlock() + + region := topo.GetRegionByCell(context.Background(), tc.ts, cell) + tc.cellRegions[cell] = region + + return region } // StatsUpdate is part of the HealthCheckStatsListener interface. func (tc *TabletStatsCache) StatsUpdate(ts *TabletStats) { - if ts.Target.TabletType != topodatapb.TabletType_MASTER && ts.Tablet.Alias.Cell != tc.cell && tc.getRegionByCell(ts.Tablet.Alias.Cell) != tc.getRegionByCell(tc.cell) { + if ts.Target.TabletType != topodatapb.TabletType_MASTER && + ts.Tablet.Alias.Cell != tc.cell && + tc.getOrCreateRegionByCell(ts.Tablet.Alias.Cell) != tc.getOrCreateRegionByCell(tc.cell) { // this is for a non-master tablet in a different cell and a different region, drop it return } @@ -270,7 +296,7 @@ func (tc *TabletStatsCache) StatsUpdate(ts *TabletStats) { func (tc *TabletStatsCache) makeAggregateMap(stats []*TabletStats) map[string]*querypb.AggregateStats { result := make(map[string]*querypb.AggregateStats) for _, ts := range stats { - region := tc.getRegionByCell(ts.Tablet.Alias.Cell) + region := tc.getOrCreateRegionByCell(ts.Tablet.Alias.Cell) agg, ok := result[region] if !ok { agg = &querypb.AggregateStats{ @@ -363,7 +389,7 @@ func (tc *TabletStatsCache) GetAggregateStats(target *querypb.Target) (*querypb. return agg, nil } } - targetRegion := tc.getRegionByCell(target.Cell) + targetRegion := tc.getOrCreateRegionByCell(target.Cell) agg, ok := e.aggregates[targetRegion] if !ok { return nil, topo.NewError(topo.NoNode, topotools.TargetIdent(target)) From f49d9e07c357e0ae8a4e762aa4e56b29247cf8ed Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Wed, 13 Feb 2019 16:44:48 -0500 Subject: [PATCH 2/3] Have a single method for region Signed-off-by: Leo Xuzhang Lin --- go/vt/discovery/tablet_stats_cache.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/go/vt/discovery/tablet_stats_cache.go b/go/vt/discovery/tablet_stats_cache.go index f3bbe453ea2..db8b6780bce 100644 --- a/go/vt/discovery/tablet_stats_cache.go +++ b/go/vt/discovery/tablet_stats_cache.go @@ -196,25 +196,14 @@ func (tc *TabletStatsCache) getOrCreateEntry(target *querypb.Target) *tabletStat return e } -func (tc *TabletStatsCache) getRegionByCell(cell string) (string, bool) { - tc.mu.RLock() - defer tc.mu.RUnlock() - if region, ok := tc.cellRegions[cell]; ok { - return region, true - } - return "", false -} +func (tc *TabletStatsCache) getRegionByCell(cell string) string { + tc.mu.Lock() + defer tc.mu.Unlock() -func (tc *TabletStatsCache) getOrCreateRegionByCell(cell string) string { - // Fast path - if region, ok := tc.getRegionByCell(cell); ok { + if region, ok := tc.cellRegions[cell]; ok { return region } - // Slow path - tc.mu.Lock() - defer tc.mu.Unlock() - region := topo.GetRegionByCell(context.Background(), tc.ts, cell) tc.cellRegions[cell] = region @@ -225,7 +214,7 @@ func (tc *TabletStatsCache) getOrCreateRegionByCell(cell string) string { func (tc *TabletStatsCache) StatsUpdate(ts *TabletStats) { if ts.Target.TabletType != topodatapb.TabletType_MASTER && ts.Tablet.Alias.Cell != tc.cell && - tc.getOrCreateRegionByCell(ts.Tablet.Alias.Cell) != tc.getOrCreateRegionByCell(tc.cell) { + tc.getRegionByCell(ts.Tablet.Alias.Cell) != tc.getRegionByCell(tc.cell) { // this is for a non-master tablet in a different cell and a different region, drop it return } @@ -296,7 +285,7 @@ func (tc *TabletStatsCache) StatsUpdate(ts *TabletStats) { func (tc *TabletStatsCache) makeAggregateMap(stats []*TabletStats) map[string]*querypb.AggregateStats { result := make(map[string]*querypb.AggregateStats) for _, ts := range stats { - region := tc.getOrCreateRegionByCell(ts.Tablet.Alias.Cell) + region := tc.getRegionByCell(ts.Tablet.Alias.Cell) agg, ok := result[region] if !ok { agg = &querypb.AggregateStats{ @@ -389,7 +378,7 @@ func (tc *TabletStatsCache) GetAggregateStats(target *querypb.Target) (*querypb. return agg, nil } } - targetRegion := tc.getOrCreateRegionByCell(target.Cell) + targetRegion := tc.getRegionByCell(target.Cell) agg, ok := e.aggregates[targetRegion] if !ok { return nil, topo.NewError(topo.NoNode, topotools.TargetIdent(target)) From 64c7c6101f462fb0e339e5ee8ded12424cc3a27e Mon Sep 17 00:00:00 2001 From: Leo Xuzhang Lin Date: Wed, 13 Feb 2019 17:52:04 -0500 Subject: [PATCH 3/3] Alloc cellRegions in test Signed-off-by: Leo Xuzhang Lin --- go/vt/discovery/tablet_stats_cache_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/go/vt/discovery/tablet_stats_cache_test.go b/go/vt/discovery/tablet_stats_cache_test.go index cfc9ba21d3b..8bc5e8caafd 100644 --- a/go/vt/discovery/tablet_stats_cache_test.go +++ b/go/vt/discovery/tablet_stats_cache_test.go @@ -38,8 +38,9 @@ func TestTabletStatsCache(t *testing.T) { // HealthCheck object, so we can't call NewTabletStatsCache. // So we just construct this object here. tsc := &TabletStatsCache{ - cell: "cell", - entries: make(map[string]map[string]map[topodatapb.TabletType]*tabletStatsCacheEntry), + cell: "cell", + entries: make(map[string]map[string]map[topodatapb.TabletType]*tabletStatsCacheEntry), + cellRegions: make(map[string]string), } // empty