From e8497f7bf69cdcb60555ecb1c3b7af36e9f4f55f Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Fri, 10 Apr 2020 17:30:15 -0700 Subject: [PATCH 1/7] Refactor EnsureVSchema to take in a cell Signed-off-by: Maggie Zhou --- go/vt/topo/shard.go | 4 ++-- go/vt/topo/vschema.go | 5 +++-- go/vt/vtctl/vtctl.go | 7 ++++++- go/vt/vttablet/tabletmanager/init_tablet.go | 3 ++- go/vt/wrangler/tablet.go | 2 +- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/go/vt/topo/shard.go b/go/vt/topo/shard.go index 7d4757742b8..ace57ccd5ca 100644 --- a/go/vt/topo/shard.go +++ b/go/vt/topo/shard.go @@ -325,7 +325,7 @@ func (ts *Server) CreateShard(ctx context.Context, keyspace, shard string) (err // GetOrCreateShard will return the shard object, or create one if it doesn't // already exist. Note the shard creation is protected by a keyspace Lock. -func (ts *Server) GetOrCreateShard(ctx context.Context, keyspace, shard string) (si *ShardInfo, err error) { +func (ts *Server) GetOrCreateShard(ctx context.Context, keyspace, shard, cell string) (si *ShardInfo, err error) { log.Info("GetShard %s/%s", keyspace, shard) si, err = ts.GetShard(ctx, keyspace, shard) if !IsErrType(err, NoNode) { @@ -340,7 +340,7 @@ func (ts *Server) GetOrCreateShard(ctx context.Context, keyspace, shard string) // make sure a valid vschema has been loaded log.Info("EnsureVSchema %s/%s", keyspace, shard) - if err = ts.EnsureVSchema(ctx, keyspace); err != nil { + if err = ts.EnsureVSchema(ctx, keyspace, []string{cell}); err != nil { return nil, vterrors.Wrapf(err, "EnsureVSchema(%v) failed", keyspace) } diff --git a/go/vt/topo/vschema.go b/go/vt/topo/vschema.go index 00e5b2ea42b..94b7db2ec2c 100644 --- a/go/vt/topo/vschema.go +++ b/go/vt/topo/vschema.go @@ -72,7 +72,7 @@ func (ts *Server) GetVSchema(ctx context.Context, keyspace string) (*vschemapb.K } // EnsureVSchema makes sure that a vschema is present for this keyspace or creates a blank one if it is missing -func (ts *Server) EnsureVSchema(ctx context.Context, keyspace string) error { +func (ts *Server) EnsureVSchema(ctx context.Context, keyspace string, cells []string) error { vschema, err := ts.GetVSchema(ctx, keyspace) if err != nil && !IsErrType(err, NoNode) { log.Info("error in getting vschema for keyspace %s: %v", keyspace, err) @@ -89,11 +89,12 @@ func (ts *Server) EnsureVSchema(ctx context.Context, keyspace string) error { } } - err = ts.RebuildSrvVSchema(ctx, []string{} /* cells */) + err = ts.RebuildSrvVSchema(ctx, cells) if err != nil { log.Errorf("could not rebuild SrvVschema after creating keyspace: %v", err) return err } + return nil } diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 4622dfbdb6c..f0397b979fb 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -1620,7 +1620,12 @@ func commandCreateKeyspace(ctx context.Context, wr *wrangler.Wrangler, subFlags } if !*allowEmptyVSchema { - err = wr.TopoServer().EnsureVSchema(ctx, keyspace) + cells, err := wr.TopoServer().GetKnownCells(ctx) + if err != nil { + return fmt.Errorf("GetKnownCells failed: %v", err) + } + + err = wr.TopoServer().EnsureVSchema(ctx, keyspace, cells) } if err != nil { diff --git a/go/vt/vttablet/tabletmanager/init_tablet.go b/go/vt/vttablet/tabletmanager/init_tablet.go index f65a4864e15..cb029fa854f 100644 --- a/go/vt/vttablet/tabletmanager/init_tablet.go +++ b/go/vt/vttablet/tabletmanager/init_tablet.go @@ -90,7 +90,8 @@ func (agent *ActionAgent) InitTablet(port, gRPCPort int32) error { var si *topo.ShardInfo if err := agent.withRetry(ctx, "creating keyspace and shard", func() error { var err error - si, err = agent.TopoServer.GetOrCreateShard(ctx, *initKeyspace, shard) + cell := agent.TabletAlias.GetCell() + si, err = agent.TopoServer.GetOrCreateShard(ctx, *initKeyspace, shard, cell) return err }); err != nil { return vterrors.Wrap(err, "InitTablet cannot GetOrCreateShard shard") diff --git a/go/vt/wrangler/tablet.go b/go/vt/wrangler/tablet.go index 82075ecbb05..a98a05464b4 100644 --- a/go/vt/wrangler/tablet.go +++ b/go/vt/wrangler/tablet.go @@ -53,7 +53,7 @@ func (wr *Wrangler) InitTablet(ctx context.Context, tablet *topodatapb.Tablet, a if createShardAndKeyspace { // create the parent keyspace and shard if needed - si, err = wr.ts.GetOrCreateShard(ctx, tablet.Keyspace, tablet.Shard) + si, err = wr.ts.GetOrCreateShard(ctx, tablet.Keyspace, tablet.Shard, tablet.Alias.GetCell()) } else { si, err = wr.ts.GetShard(ctx, tablet.Keyspace, tablet.Shard) if topo.IsErrType(err, topo.NoNode) { From 2a50906d76f23c344dc3f83b54693e4692ba19a6 Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Mon, 13 Apr 2020 09:57:49 -0700 Subject: [PATCH 2/7] Add cell to GetOrCreateShard to the test files too. Signed-off-by: Maggie Zhou --- go/vt/topotools/shard_test.go | 5 +++-- go/vt/wrangler/testlib/reparent_utils_test.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/go/vt/topotools/shard_test.go b/go/vt/topotools/shard_test.go index 68ab7be9a99..4717588d968 100644 --- a/go/vt/topotools/shard_test.go +++ b/go/vt/topotools/shard_test.go @@ -105,7 +105,8 @@ func TestGetOrCreateShard(t *testing.T) { ctx := context.Background() // Set up topology. - ts := memorytopo.NewServer("test_cell") + cell := "test_cell" + ts := memorytopo.NewServer(cell) // and do massive parallel GetOrCreateShard keyspace := "test_keyspace" @@ -119,7 +120,7 @@ func TestGetOrCreateShard(t *testing.T) { for j := 0; j < 100; j++ { index := rand.Intn(10) shard := fmt.Sprintf("%v", index) - si, err := ts.GetOrCreateShard(ctx, keyspace, shard) + si, err := ts.GetOrCreateShard(ctx, keyspace, shard, cell) if err != nil { t.Errorf("GetOrCreateShard(%v, %v) failed: %v", i, shard, err) } diff --git a/go/vt/wrangler/testlib/reparent_utils_test.go b/go/vt/wrangler/testlib/reparent_utils_test.go index 109707c6a48..1e85f0c2e29 100644 --- a/go/vt/wrangler/testlib/reparent_utils_test.go +++ b/go/vt/wrangler/testlib/reparent_utils_test.go @@ -38,7 +38,7 @@ func TestShardReplicationStatuses(t *testing.T) { wr := wrangler.New(logutil.NewConsoleLogger(), ts, tmclient.NewTabletManagerClient()) // create shard and tablets - if _, err := ts.GetOrCreateShard(ctx, "test_keyspace", "0"); err != nil { + if _, err := ts.GetOrCreateShard(ctx, "test_keyspace", "0", "cell1"); err != nil { t.Fatalf("GetOrCreateShard failed: %v", err) } master := NewFakeTablet(t, wr, "cell1", 1, topodatapb.TabletType_MASTER, nil) @@ -108,7 +108,7 @@ func TestReparentTablet(t *testing.T) { wr := wrangler.New(logutil.NewConsoleLogger(), ts, tmclient.NewTabletManagerClient()) // create shard and tablets - if _, err := ts.GetOrCreateShard(ctx, "test_keyspace", "0"); err != nil { + if _, err := ts.GetOrCreateShard(ctx, "test_keyspace", "0", "cell1"); err != nil { t.Fatalf("CreateShard failed: %v", err) } master := NewFakeTablet(t, wr, "cell1", 1, topodatapb.TabletType_MASTER, nil) From 1724a7165ed230e07e5429e11fed72ef98512539 Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Tue, 14 Apr 2020 10:52:31 -0700 Subject: [PATCH 3/7] Remove the nil check on CreateKeyspace to see if it lets you keep going Signed-off-by: Maggie Zhou --- go/test/endtoend/recovery/recovery_util.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/test/endtoend/recovery/recovery_util.go b/go/test/endtoend/recovery/recovery_util.go index cd6c82e8a3b..9bb12128eb8 100644 --- a/go/test/endtoend/recovery/recovery_util.go +++ b/go/test/endtoend/recovery/recovery_util.go @@ -54,7 +54,6 @@ func RestoreTablet(t *testing.T, localCluster *cluster.LocalProcessCluster, tabl _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) - require.Nil(t, err) replicaTabletArgs := commonTabletArg if UseXb { From 5c29a6b46249d71410c448803a434dc80643ba3e Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Wed, 15 Apr 2020 12:48:09 -0700 Subject: [PATCH 4/7] Respond to code review. Add back in the nil check for CreateKeyspace. Remove empty line. Signed-off-by: Maggie Zhou --- go/test/endtoend/recovery/recovery_util.go | 1 + go/vt/topo/vschema.go | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/go/test/endtoend/recovery/recovery_util.go b/go/test/endtoend/recovery/recovery_util.go index 9bb12128eb8..cd6c82e8a3b 100644 --- a/go/test/endtoend/recovery/recovery_util.go +++ b/go/test/endtoend/recovery/recovery_util.go @@ -54,6 +54,7 @@ func RestoreTablet(t *testing.T, localCluster *cluster.LocalProcessCluster, tabl _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) + require.Nil(t, err) replicaTabletArgs := commonTabletArg if UseXb { diff --git a/go/vt/topo/vschema.go b/go/vt/topo/vschema.go index 94b7db2ec2c..40837595c8d 100644 --- a/go/vt/topo/vschema.go +++ b/go/vt/topo/vschema.go @@ -94,7 +94,6 @@ func (ts *Server) EnsureVSchema(ctx context.Context, keyspace string, cells []st log.Errorf("could not rebuild SrvVschema after creating keyspace: %v", err) return err } - return nil } From ca85789e4c2da7fea80f4eeef12b020b1ab2dbc8 Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Wed, 15 Apr 2020 21:45:59 -0700 Subject: [PATCH 5/7] conditionally createkeyspace if it doesn't exist already. Signed-off-by: Maggie Zhou --- go/test/endtoend/recovery/recovery_util.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/go/test/endtoend/recovery/recovery_util.go b/go/test/endtoend/recovery/recovery_util.go index cd6c82e8a3b..415bc226e1b 100644 --- a/go/test/endtoend/recovery/recovery_util.go +++ b/go/test/endtoend/recovery/recovery_util.go @@ -51,10 +51,13 @@ func RestoreTablet(t *testing.T, localCluster *cluster.LocalProcessCluster, tabl tablet.ValidateTabletRestart(t) tm := time.Now().UTC() tm.Format(time.RFC3339) - _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", - "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, - "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) - require.Nil(t, err) + _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("GetKeyspace", keyspaceName) + if err != nil { + _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", + "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, + "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) + require.Nil(t, err) + } replicaTabletArgs := commonTabletArg if UseXb { From 72e75c18e8aa6b2853166e653bed92956e6f22ed Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Wed, 15 Apr 2020 23:43:29 -0700 Subject: [PATCH 6/7] ok try with the flaky test Signed-off-by: Maggie Zhou --- go/test/endtoend/recovery/recovery_util.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/go/test/endtoend/recovery/recovery_util.go b/go/test/endtoend/recovery/recovery_util.go index 415bc226e1b..cd6c82e8a3b 100644 --- a/go/test/endtoend/recovery/recovery_util.go +++ b/go/test/endtoend/recovery/recovery_util.go @@ -51,13 +51,10 @@ func RestoreTablet(t *testing.T, localCluster *cluster.LocalProcessCluster, tabl tablet.ValidateTabletRestart(t) tm := time.Now().UTC() tm.Format(time.RFC3339) - _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("GetKeyspace", keyspaceName) - if err != nil { - _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", - "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, - "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) - require.Nil(t, err) - } + _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", + "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, + "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) + require.Nil(t, err) replicaTabletArgs := commonTabletArg if UseXb { From f8edec43d5eaa6b743bd5521be36e99fc9a9b66f Mon Sep 17 00:00:00 2001 From: Maggie Zhou Date: Thu, 16 Apr 2020 12:49:40 -0700 Subject: [PATCH 7/7] If the keyspace already exists, don't try to create it again, it'll throw an error Signed-off-by: Maggie Zhou --- go/test/endtoend/recovery/recovery_util.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/go/test/endtoend/recovery/recovery_util.go b/go/test/endtoend/recovery/recovery_util.go index cd6c82e8a3b..054ef63127f 100644 --- a/go/test/endtoend/recovery/recovery_util.go +++ b/go/test/endtoend/recovery/recovery_util.go @@ -49,14 +49,19 @@ func VerifyQueriesUsingVtgate(t *testing.T, session *vtgateconn.VTGateSession, q func RestoreTablet(t *testing.T, localCluster *cluster.LocalProcessCluster, tablet *cluster.Vttablet, restoreKSName string, shardName string, keyspaceName string, commonTabletArg []string) { tablet.ValidateTabletRestart(t) - tm := time.Now().UTC() - tm.Format(time.RFC3339) - _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", - "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, - "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) - require.Nil(t, err) - replicaTabletArgs := commonTabletArg + + _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("GetKeyspace", restoreKSName) + + if err != nil { + tm := time.Now().UTC() + tm.Format(time.RFC3339) + _, err := localCluster.VtctlProcess.ExecuteCommandWithOutput("CreateKeyspace", + "-keyspace_type=SNAPSHOT", "-base_keyspace="+keyspaceName, + "-snapshot_time", tm.Format(time.RFC3339), restoreKSName) + require.Nil(t, err) + } + if UseXb { replicaTabletArgs = append(replicaTabletArgs, XbArgs...) }