Merge pull request #4631 from tinyspeck/4496-topo-serving-shards-refa…

…ctor

4496 topo serving shards refactor

go/cmd/vtcombo/main.go

@@ -33,11 +33,13 @@ import (
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/discovery"
 	"vitess.io/vitess/go/vt/log"
+	"vitess.io/vitess/go/vt/logutil"
 	"vitess.io/vitess/go/vt/mysqlctl"
 	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/srvtopo"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/memorytopo"
+	"vitess.io/vitess/go/vt/topotools"
 	"vitess.io/vitess/go/vt/vtcombo"
 	"vitess.io/vitess/go/vt/vtctld"
 	"vitess.io/vitess/go/vt/vtgate"
@@ -108,6 +110,14 @@ func main() {
 		exit.Return(1)
 	}
 
+	// Now that we have fully initialized the tablets, rebuild the keyspace graph. This is what would normally happen in InitTablet.
+	for _, ks := range tpb.Keyspaces {
+		err := topotools.RebuildKeyspace(context.Background(), logutil.NewConsoleLogger(), ts, ks.GetName(), tpb.Cells)
+		if err != nil {
+			log.Fatalf("Couldn't build srv keyspace for (%v: %v). Got error: %v", ks, tpb.Cells, err)
+		}
+	}
+
 	// vtgate configuration and init
 	resilientServer := srvtopo.NewResilientServer(ts, "ResilientSrvTopoServer")
 	healthCheck := discovery.NewHealthCheck(1*time.Millisecond /*retryDelay*/, 1*time.Hour /*healthCheckTimeout*/)

go/vt/srvtopo/resolver_test.go

@@ -24,7 +24,6 @@ import (
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/key"
 	"vitess.io/vitess/go/vt/logutil"
-	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/memorytopo"
 	"vitess.io/vitess/go/vt/topotools"
 	"vitess.io/vitess/go/vt/vttablet/queryservice"
@@ -63,12 +62,6 @@ func initResolver(t *testing.T, name string) *Resolver {
 		if err := ts.CreateShard(ctx, "sks", shard); err != nil {
 			t.Fatalf("CreateShard(\"%v\") failed: %v", shard, err)
 		}
-		if _, err := ts.UpdateShardFields(ctx, "sks", shard, func(si *topo.ShardInfo) error {
-			si.Shard.Cells = []string{"cell1"}
-			return nil
-		}); err != nil {
-			t.Fatalf("UpdateShardFields(\"%v\") failed: %v", shard, err)
-		}
 	}
 
 	// Create unsharded keyspace and shard.
@@ -78,12 +71,6 @@ func initResolver(t *testing.T, name string) *Resolver {
 	if err := ts.CreateShard(ctx, "uks", "0"); err != nil {
 		t.Fatalf("CreateShard(0) failed: %v", err)
 	}
-	if _, err := ts.UpdateShardFields(ctx, "uks", "0", func(si *topo.ShardInfo) error {
-		si.Shard.Cells = []string{"cell1"}
-		return nil
-	}); err != nil {
-		t.Fatalf("UpdateShardFields(0) failed: %v", err)
-	}
 
 	// And rebuild both.
 	for _, keyspace := range []string{"sks", "uks"} {

go/vt/topo/cell_info.go

@@ -17,11 +17,11 @@ limitations under the License.
 package topo
 
 import (
+	"fmt"
 	"path"
 
 	"github.com/golang/protobuf/proto"
 	"golang.org/x/net/context"
-	"vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/vterrors"
 
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
@@ -137,24 +137,16 @@ func (ts *Server) UpdateCellInfoFields(ctx context.Context, cell string, update
 // DeleteCellInfo deletes the specified CellInfo.
 // We first make sure no Shard record points to the cell.
 func (ts *Server) DeleteCellInfo(ctx context.Context, cell string) error {
-	// Get all keyspaces.
-	keyspaces, err := ts.GetKeyspaces(ctx)
-	if err != nil {
-		return vterrors.Wrap(err, "GetKeyspaces() failed")
-	}
-
-	// For each keyspace, make sure no shard points at the cell.
-	for _, keyspace := range keyspaces {
-		shards, err := ts.FindAllShardsInKeyspace(ctx, keyspace)
-		if err != nil {
-			return vterrors.Wrapf(err, "FindAllShardsInKeyspace(%v) failed", keyspace)
-		}
-
-		for shard, si := range shards {
-			if si.HasCell(cell) {
-				return vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "cell %v is used by shard %v/%v, cannot remove it. Use 'vtctl RemoveShardCell' to remove unused cells in a Shard", cell, keyspace, shard)
-			}
+	srvKeyspaces, err := ts.GetSrvKeyspaceNames(ctx, cell)
+	switch {
+	case err == nil:
+		if len(srvKeyspaces) != 0 {
+			return vterrors.Wrap(err, fmt.Sprintf("cell %v has serving keyspaces. Before deleting, delete keyspace with: DeleteKeyspace", cell))
 		}
+	case IsErrType(err, NoNode):
+		// Nothing to do.
+	default:
+		return vterrors.Wrap(err, "GetSrvKeyspaceNames() failed")
 	}
 
 	filePath := pathForCellInfo(cell)

go/vt/topo/helpers/compare.go

@@ -146,6 +146,10 @@ func CompareShardReplications(ctx context.Context, fromTS, toTS *topo.Server) er
 	if err != nil {
 		return vterrors.Wrapf(err, "fromTS.GetKeyspaces")
 	}
+	cells, err := fromTS.GetCellInfoNames(ctx)
+	if err != nil {
+		return vterrors.Wrap(err, "GetCellInfoNames()")
+	}
 
 	for _, keyspace := range keyspaces {
 		shards, err := fromTS.GetShardNames(ctx, keyspace)
@@ -154,14 +158,7 @@ func CompareShardReplications(ctx context.Context, fromTS, toTS *topo.Server) er
 		}
 
 		for _, shard := range shards {
-
-			// read the source shard to get the cells
-			si, err := fromTS.GetShard(ctx, keyspace, shard)
-			if err != nil {
-				return vterrors.Wrapf(err, "GetShard(%v, %v)", keyspace, shard)
-			}
-
-			for _, cell := range si.Shard.Cells {
+			for _, cell := range cells {
 				fromSRi, err := fromTS.GetShardReplication(ctx, cell, keyspace, shard)
 				if err != nil {
 					return vterrors.Wrapf(err, "GetShardReplication(%v, %v, %v)", cell, keyspace, shard)

go/vt/topo/helpers/copy.go

@@ -146,21 +146,19 @@ func CopyShardReplications(ctx context.Context, fromTS, toTS *topo.Server) {
 		log.Fatalf("fromTS.GetKeyspaces: %v", err)
 	}
 
+	cells, err := fromTS.GetCellInfoNames(ctx)
+	if err != nil {
+		log.Fatalf("GetCellInfoNames(): %v", err)
+	}
+
 	for _, keyspace := range keyspaces {
 		shards, err := fromTS.GetShardNames(ctx, keyspace)
 		if err != nil {
 			log.Fatalf("GetShardNames(%v): %v", keyspace, err)
 		}
 
 		for _, shard := range shards {
-
-			// read the source shard to get the cells
-			si, err := fromTS.GetShard(ctx, keyspace, shard)
-			if err != nil {
-				log.Fatalf("GetShard(%v, %v): %v", keyspace, shard, err)
-			}
-
-			for _, cell := range si.Shard.Cells {
+			for _, cell := range cells {
 				sri, err := fromTS.GetShardReplication(ctx, cell, keyspace, shard)
 				if err != nil {
 					log.Fatalf("GetShardReplication(%v, %v, %v): %v", cell, keyspace, shard, err)

go/vt/topo/helpers/copy_test.go

@@ -42,12 +42,6 @@ func createSetup(ctx context.Context, t *testing.T) (*topo.Server, *topo.Server)
 	if err := fromTS.CreateShard(ctx, "test_keyspace", "0"); err != nil {
 		t.Fatalf("cannot create shard: %v", err)
 	}
-	if _, err := fromTS.UpdateShardFields(ctx, "test_keyspace", "0", func(si *topo.ShardInfo) error {
-		si.Cells = []string{"test_cell"}
-		return nil
-	}); err != nil {
-		t.Fatalf("cannot update shard: %v", err)
-	}
 	tablet1 := &topodatapb.Tablet{
 		Alias: &topodatapb.TabletAlias{
 			Cell: "test_cell",
@@ -120,13 +114,6 @@ func TestBasic(t *testing.T) {
 		t.Fatalf("unexpected shards: %v", shards)
 	}
 	CopyShards(ctx, fromTS, toTS)
-	si, err := toTS.GetShard(ctx, "test_keyspace", "0")
-	if err != nil {
-		t.Fatalf("cannot read shard: %v", err)
-	}
-	if len(si.Shard.Cells) != 1 || si.Shard.Cells[0] != "test_cell" {
-		t.Fatalf("bad shard data: %v", *si.Shard)
-	}
 
 	// check ShardReplication copy
 	sr, err := fromTS.GetShardReplication(ctx, "test_cell", "test_keyspace", "0")

go/vt/topo/server.go

@@ -44,6 +44,7 @@ package topo
 
 import (
 	"flag"
+	"fmt"
 	"sync"
 
 	"golang.org/x/net/context"
@@ -257,12 +258,17 @@ func (ts *Server) ConnForCell(ctx context.Context, cell string) (Conn, error) {
 
 	// Create the connection.
 	conn, err = ts.factory.Create(cell, ci.ServerAddress, ci.Root)
-	if err != nil {
-		return nil, vterrors.Wrapf(err, "failed to create topo connection to %v, %v", ci.ServerAddress, ci.Root)
+	switch {
+	case err == nil:
+		conn = NewStatsConn(cell, conn)
+		ts.cells[cell] = conn
+		return conn, nil
+	case IsErrType(err, NoNode):
+		err = vterrors.Wrap(err, fmt.Sprintf("failed to create topo connection to %v, %v", ci.ServerAddress, ci.Root))
+		return nil, NewError(NoNode, err.Error())
+	default:
+		return nil, vterrors.Wrap(err, fmt.Sprintf("failed to create topo connection to %v, %v", ci.ServerAddress, ci.Root))
 	}
-	conn = NewStatsConn(cell, conn)
-	ts.cells[cell] = conn
-	return conn, nil
 }
 
 // GetRegionByCell returns the region group this `cell` belongs to, if there's none, it returns the `cell` as region.