Slack vitess 2019 04 02.r0

go/sqltypes/result.go

@@ -185,13 +185,14 @@ func ResultsEqual(r1, r2 []Result) bool {
 // Every place this function is called, a comment is needed that explains
 // why it's justified.
 func MakeRowTrusted(fields []*querypb.Field, row *querypb.Row) []Value {
-	sqlRow := make([]Value, len(row.Lengths))
+	sqlRow := make([]Value, len(fields))
 	var offset int64
-	for i, length := range row.Lengths {
+	for i, fld := range fields {
+		length := row.Lengths[i]
 		if length < 0 {
 			continue
 		}
-		sqlRow[i] = MakeTrusted(fields[i].Type, row.Values[offset:offset+length])
+		sqlRow[i] = MakeTrusted(fld.Type, row.Values[offset:offset+length])
 		offset += length
 	}
 	return sqlRow

go/sqltypes/result_test.go

@@ -23,6 +23,61 @@ import (
 	querypb "vitess.io/vitess/go/vt/proto/query"
 )
 
+func TestMakeRowTrusted(t *testing.T) {
+	fields := MakeTestFields(
+		"some_int|some_text|another_int",
+		"int8|varchar|int8",
+	)
+
+	values := []byte{}
+	hw := []byte("hello, world")
+	values = append(values, hw...)
+	values = append(values, byte(42))
+
+	row := &querypb.Row{
+		Lengths: []int64{-1, int64(len(hw)), 1},
+		Values:  values,
+	}
+
+	want := []Value{
+		MakeTrusted(querypb.Type_NULL_TYPE, nil),
+		MakeTrusted(querypb.Type_VARCHAR, []byte("hello, world")),
+		MakeTrusted(querypb.Type_INT8, []byte{byte(42)}),
+	}
+
+	result := MakeRowTrusted(fields, row)
+	if !reflect.DeepEqual(result, want) {
+		t.Errorf("MakeRowTrusted:\ngot: %#v\nwant: %#v", result, want)
+	}
+}
+
+func TestMakeRowTrustedDoesNotPanicOnNewColumns(t *testing.T) {
+	fields := MakeTestFields(
+		"some_int|some_text",
+		"int8|varchar",
+	)
+
+	values := []byte{byte(123)}
+	hw := []byte("hello, world")
+	values = append(values, hw...)
+	values = append(values, byte(42))
+
+	row := &querypb.Row{
+		Lengths: []int64{1, int64(len(hw)), 1},
+		Values:  values,
+	}
+
+	want := []Value{
+		MakeTrusted(querypb.Type_INT8, []byte{byte(123)}),
+		MakeTrusted(querypb.Type_VARCHAR, []byte("hello, world")),
+	}
+
+	result := MakeRowTrusted(fields, row)
+	if !reflect.DeepEqual(result, want) {
+		t.Errorf("MakeRowTrusted:\ngot: %#v\nwant: %#v", result, want)
+	}
+}
+
 func TestRepair(t *testing.T) {
 	fields := []*querypb.Field{{
 		Type: Int64,

go/vt/discovery/tablet_stats_cache.go

@@ -56,8 +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
+	// cellAliases is a cache of cell aliases
+	cellAliases map[string]string
 }
 
 // tabletStatsCacheEntry is the per keyspace/shard/tabletType
@@ -70,7 +70,7 @@ type tabletStatsCacheEntry struct {
 	all map[string]*TabletStats
 	// healthy only has the healthy ones.
 	healthy []*TabletStats
-	// aggregates has the per-region aggregates.
+	// aggregates has the per-alias aggregates.
 	aggregates map[string]*querypb.AggregateStats
 }
 
@@ -136,7 +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),
+		cellAliases:    make(map[string]string),
 	}
 
 	if setListener {
@@ -196,26 +196,26 @@ func (tc *TabletStatsCache) getOrCreateEntry(target *querypb.Target) *tabletStat
 	return e
 }
 
-func (tc *TabletStatsCache) getRegionByCell(cell string) string {
+func (tc *TabletStatsCache) getAliasByCell(cell string) string {
 	tc.mu.Lock()
 	defer tc.mu.Unlock()
 
-	if region, ok := tc.cellRegions[cell]; ok {
-		return region
+	if alias, ok := tc.cellAliases[cell]; ok {
+		return alias
 	}
 
-	region := topo.GetRegionByCell(context.Background(), tc.ts, cell)
-	tc.cellRegions[cell] = region
+	alias := topo.GetAliasByCell(context.Background(), tc.ts, cell)
+	tc.cellAliases[cell] = alias
 
-	return region
+	return alias
 }
 
 // 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) {
-		// this is for a non-master tablet in a different cell and a different region, drop it
+		tc.getAliasByCell(ts.Tablet.Alias.Cell) != tc.getAliasByCell(tc.cell) {
+		// this is for a non-master tablet in a different cell and a different alias, drop it
 		return
 	}
 
@@ -280,18 +280,18 @@ func (tc *TabletStatsCache) StatsUpdate(ts *TabletStats) {
 	tc.updateAggregateMap(ts.Target.Keyspace, ts.Target.Shard, ts.Target.TabletType, e, allArray)
 }
 
-// makeAggregateMap takes a list of TabletStats and builds a per-region
+// makeAggregateMap takes a list of TabletStats and builds a per-alias
 // AggregateStats map.
 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)
-		agg, ok := result[region]
+		alias := tc.getAliasByCell(ts.Tablet.Alias.Cell)
+		agg, ok := result[alias]
 		if !ok {
 			agg = &querypb.AggregateStats{
 				SecondsBehindMasterMin: math.MaxUint32,
 			}
-			result[region] = agg
+			result[alias] = agg
 		}
 
 		if ts.Serving && ts.LastError == nil {
@@ -378,8 +378,8 @@ func (tc *TabletStatsCache) GetAggregateStats(target *querypb.Target) (*querypb.
 			return agg, nil
 		}
 	}
-	targetRegion := tc.getRegionByCell(target.Cell)
-	agg, ok := e.aggregates[targetRegion]
+	targetAlias := tc.getAliasByCell(target.Cell)
+	agg, ok := e.aggregates[targetAlias]
 	if !ok {
 		return nil, topo.NewError(topo.NoNode, topotools.TargetIdent(target))
 	}

go/vt/discovery/tablet_stats_cache_test.go

@@ -17,30 +17,44 @@ limitations under the License.
 package discovery
 
 import (
+	"context"
 	"testing"
 
 	"vitess.io/vitess/go/vt/topo"
+	"vitess.io/vitess/go/vt/topo/memorytopo"
 
 	querypb "vitess.io/vitess/go/vt/proto/query"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 )
 
 // TestTabletStatsCache tests the functionality of the TabletStatsCache class.
 func TestTabletStatsCache(t *testing.T) {
-	defer topo.UpdateCellsToRegionsForTests(map[string]string{})
-	topo.UpdateCellsToRegionsForTests(map[string]string{
-		"cell":  "region1",
-		"cell1": "region1",
-		"cell2": "region2",
-	})
+	ts := memorytopo.NewServer("cell", "cell1", "cell2")
+
+	cellsAlias := &topodatapb.CellsAlias{
+		Cells: []string{"cell", "cell1"},
+	}
+
+	ts.CreateCellsAlias(context.Background(), "region1", cellsAlias)
+
+	defer ts.DeleteCellsAlias(context.Background(), "region1")
+
+	cellsAlias = &topodatapb.CellsAlias{
+		Cells: []string{"cell2"},
+	}
+
+	ts.CreateCellsAlias(context.Background(), "region2", cellsAlias)
+
+	defer ts.DeleteCellsAlias(context.Background(), "region2")
 
 	// We want to unit test TabletStatsCache without a full-blown
 	// HealthCheck object, so we can't call NewTabletStatsCache.
 	// So we just construct this object here.
 	tsc := &TabletStatsCache{
 		cell:        "cell",
+		ts:          ts,
 		entries:     make(map[string]map[string]map[topodatapb.TabletType]*tabletStatsCacheEntry),
-		cellRegions: make(map[string]string),
+		cellAliases: make(map[string]string),
 	}
 
 	// empty