From 4d13b28a3ca770c3149ce737171a6d1b55205d08 Mon Sep 17 00:00:00 2001 From: deepthi Date: Thu, 27 Aug 2020 11:01:48 -0700 Subject: [PATCH 1/2] tablet picker: use partial result from GetTabletMap if it is available. check for cell before cellsAlias to reduce number of topo errors. Signed-off-by: deepthi --- go/vt/discovery/tablet_picker.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/go/vt/discovery/tablet_picker.go b/go/vt/discovery/tablet_picker.go index 27362382b9c..382888d7a9b 100644 --- a/go/vt/discovery/tablet_picker.go +++ b/go/vt/discovery/tablet_picker.go @@ -157,12 +157,17 @@ func (tp *TabletPicker) getMatchingTablets(ctx context.Context) []*topo.TabletIn // non-blocking read so that this is fast shortCtx, cancel := context.WithTimeout(ctx, *topo.RemoteOperationTimeout) defer cancel() - alias, err := tp.ts.GetCellsAlias(shortCtx, cell, false) + _, err := tp.ts.GetCellInfo(ctx, cell, false) if err != nil { - // either cellAlias doesn't exist or it isn't a cell alias at all. In that case assume it is a cell - actualCells = append(actualCells, cell) + // not a valid cell, check whether it is a cell alias + alias, err := tp.ts.GetCellsAlias(shortCtx, cell, false) + // if we get an error, either cellAlias doesn't exist or it isn't a cell alias at all. Ignore and continue + if err == nil { + actualCells = append(actualCells, alias.Cells...) + } } else { - actualCells = append(actualCells, alias.Cells...) + // valid cell, add it to our list + actualCells = append(actualCells, cell) } } for _, cell := range actualCells { @@ -171,9 +176,6 @@ func (tp *TabletPicker) getMatchingTablets(ctx context.Context) []*topo.TabletIn // match cell, keyspace and shard sri, err := tp.ts.GetShardReplication(shortCtx, cell, tp.keyspace, tp.shard) if err != nil { - //log.Errorf("missing shard in topo %s", debug.Stack()) //FIXME: remove after all tests run - - //log.Warningf("error %v from GetShardReplication for %v %v %v", err, cell, tp.keyspace, tp.shard) continue } @@ -191,14 +193,17 @@ func (tp *TabletPicker) getMatchingTablets(ctx context.Context) []*topo.TabletIn tabletMap, err := tp.ts.GetTabletMap(shortCtx, aliases) if err != nil { log.Warningf("error fetching tablets from topo: %v", err) - return nil + // If we get a partial result we can still use it, otherwise return + if len(tabletMap) == 0 { + return nil + } } tablets := make([]*topo.TabletInfo, 0, len(aliases)) for _, tabletAlias := range aliases { tabletInfo, ok := tabletMap[topoproto.TabletAliasString(tabletAlias)] if !ok { - // tablet disappeared on us (GetTabletMap ignores - // topo.ErrNoNode), just echo a warning + // Either tablet disappeared on us, or we got a partial result (GetTabletMap ignores + // topo.ErrNoNode). Just log a warning log.Warningf("failed to load tablet %v", tabletAlias) } else if topoproto.IsTypeInList(tabletInfo.Type, tp.tabletTypes) { tablets = append(tablets, tabletInfo) From 2f2faf3cb5f24609e9574e0ba09c41c5181714f1 Mon Sep 17 00:00:00 2001 From: deepthi Date: Fri, 28 Aug 2020 13:20:41 -0700 Subject: [PATCH 2/2] tablet picker: use shortCtx for all topo calls Signed-off-by: deepthi --- go/vt/discovery/tablet_picker.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/vt/discovery/tablet_picker.go b/go/vt/discovery/tablet_picker.go index 382888d7a9b..abb649c2e8a 100644 --- a/go/vt/discovery/tablet_picker.go +++ b/go/vt/discovery/tablet_picker.go @@ -157,9 +157,11 @@ func (tp *TabletPicker) getMatchingTablets(ctx context.Context) []*topo.TabletIn // non-blocking read so that this is fast shortCtx, cancel := context.WithTimeout(ctx, *topo.RemoteOperationTimeout) defer cancel() - _, err := tp.ts.GetCellInfo(ctx, cell, false) + _, err := tp.ts.GetCellInfo(shortCtx, cell, false) if err != nil { // not a valid cell, check whether it is a cell alias + shortCtx, cancel := context.WithTimeout(ctx, *topo.RemoteOperationTimeout) + defer cancel() alias, err := tp.ts.GetCellsAlias(shortCtx, cell, false) // if we get an error, either cellAlias doesn't exist or it isn't a cell alias at all. Ignore and continue if err == nil {