From 90b5c834cf2c4ace708d131598a9b0b784684b37 Mon Sep 17 00:00:00 2001 From: Jacques Grove Date: Tue, 24 Mar 2020 21:12:16 -0700 Subject: [PATCH 1/2] Issue #5265 ; add a --skip-verify option to CopySchemaShard. Signed-off-by: Jacques Grove --- go/vt/vtctl/vtctl.go | 9 +++++---- .../mock_resharding_wrangler_test.go | 6 +++--- .../resharding/resharding_wrangler.go | 2 +- go/vt/workflow/resharding/tasks.go | 2 +- go/vt/workflow/resharding/workflow_test.go | 10 +++++----- go/vt/wrangler/resharder.go | 2 +- go/vt/wrangler/schema.go | 20 ++++++++++--------- 7 files changed, 27 insertions(+), 24 deletions(-) diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 40b74e22f71..89d5586d8ee 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -400,7 +400,7 @@ var commands = []commandGroup{ "[-allow_long_unavailability] [-wait_slave_timeout=10s] {-sql= || -sql-file=} ", "Applies the schema change to the specified keyspace on every master, running in parallel on all shards. The changes are then propagated to slaves via replication. If -allow_long_unavailability is set, schema changes affecting a large number of rows (and possibly incurring a longer period of unavailability) will not be rejected."}, {"CopySchemaShard", commandCopySchemaShard, - "[-tables=,,...] [-exclude_tables=,,...] [-include-views] [-wait_slave_timeout=10s] { || } ", + "[-tables=,,...] [-exclude_tables=,,...] [-include-views] [-skip-verify] [-wait_slave_timeout=10s] { || } ", "Copies the schema from a source shard's master (or a specific tablet) to a destination shard. The schema is applied directly on the master of the destination shard, and it is propagated to the replicas through binlogs."}, {"ValidateVersionShard", commandValidateVersionShard, @@ -2305,6 +2305,7 @@ func commandCopySchemaShard(ctx context.Context, wr *wrangler.Wrangler, subFlags tables := subFlags.String("tables", "", "Specifies a comma-separated list of tables to copy. Each is either an exact match, or a regular expression of the form /regexp/") excludeTables := subFlags.String("exclude_tables", "", "Specifies a comma-separated list of tables to exclude. Each is either an exact match, or a regular expression of the form /regexp/") includeViews := subFlags.Bool("include-views", true, "Includes views in the output") + skipVerify := subFlags.Bool("skip-verify", false, "Skip verification of source and target schema after copy") waitSlaveTimeout := subFlags.Duration("wait_slave_timeout", 10*time.Second, "The amount of time to wait for slaves to receive the schema change via replication.") if err := subFlags.Parse(args); err != nil { return err @@ -2328,11 +2329,11 @@ func commandCopySchemaShard(ctx context.Context, wr *wrangler.Wrangler, subFlags sourceKeyspace, sourceShard, err := topoproto.ParseKeyspaceShard(subFlags.Arg(0)) if err == nil { - return wr.CopySchemaShardFromShard(ctx, tableArray, excludeTableArray, *includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, *waitSlaveTimeout) + return wr.CopySchemaShardFromShard(ctx, tableArray, excludeTableArray, *includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, *waitSlaveTimeout, *skipVerify) } sourceTabletAlias, err := topoproto.ParseTabletAlias(subFlags.Arg(0)) if err == nil { - return wr.CopySchemaShard(ctx, sourceTabletAlias, tableArray, excludeTableArray, *includeViews, destKeyspace, destShard, *waitSlaveTimeout) + return wr.CopySchemaShard(ctx, sourceTabletAlias, tableArray, excludeTableArray, *includeViews, destKeyspace, destShard, *waitSlaveTimeout, *skipVerify) } return err } @@ -2556,7 +2557,7 @@ func commandApplyVSchema(ctx context.Context, wr *wrangler.Wrangler, subFlags *f if _, err := wr.TopoServer().GetKeyspace(ctx, keyspace); err != nil { if strings.Contains(err.Error(), "node doesn't exist") { - return fmt.Errorf("keyspace(%s) doesn't exist, check if the keyspace is initialized.\n", keyspace) + return fmt.Errorf("keyspace(%s) doesn't exist, check if the keyspace is initialized", keyspace) } return err } diff --git a/go/vt/workflow/resharding/mock_resharding_wrangler_test.go b/go/vt/workflow/resharding/mock_resharding_wrangler_test.go index 21b68ce7f45..3b7ea4e23ba 100644 --- a/go/vt/workflow/resharding/mock_resharding_wrangler_test.go +++ b/go/vt/workflow/resharding/mock_resharding_wrangler_test.go @@ -37,15 +37,15 @@ func (m *MockReshardingWrangler) EXPECT() *MockReshardingWranglerMockRecorder { } // CopySchemaShardFromShard mocks base method -func (m *MockReshardingWrangler) CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration) error { +func (m *MockReshardingWrangler) CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration, skipVerify bool) error { ret := m.ctrl.Call(m, "CopySchemaShardFromShard", ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout) ret0, _ := ret[0].(error) return ret0 } // CopySchemaShardFromShard indicates an expected call of CopySchemaShardFromShard -func (mr *MockReshardingWranglerMockRecorder) CopySchemaShardFromShard(ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CopySchemaShardFromShard", reflect.TypeOf((*MockReshardingWrangler)(nil).CopySchemaShardFromShard), ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout) +func (mr *MockReshardingWranglerMockRecorder) CopySchemaShardFromShard(ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout interface{}, skipVerify bool) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CopySchemaShardFromShard", reflect.TypeOf((*MockReshardingWrangler)(nil).CopySchemaShardFromShard), ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout, skipVerify) } // WaitForFilteredReplication mocks base method diff --git a/go/vt/workflow/resharding/resharding_wrangler.go b/go/vt/workflow/resharding/resharding_wrangler.go index db16ca24b18..c5628637cb6 100644 --- a/go/vt/workflow/resharding/resharding_wrangler.go +++ b/go/vt/workflow/resharding/resharding_wrangler.go @@ -29,7 +29,7 @@ import ( // ReshardingWrangler is the interface to be used in creating mock interface for wrangler, which is used for unit test. It includes a subset of the methods in go/vt/Wrangler. type ReshardingWrangler interface { - CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration) error + CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration, skipVerify bool) error WaitForFilteredReplication(ctx context.Context, keyspace, shard string, maxDelay time.Duration) error diff --git a/go/vt/workflow/resharding/tasks.go b/go/vt/workflow/resharding/tasks.go index 244fcfe5801..04b2d0cc980 100644 --- a/go/vt/workflow/resharding/tasks.go +++ b/go/vt/workflow/resharding/tasks.go @@ -70,7 +70,7 @@ func (hw *horizontalReshardingWorkflow) runCopySchema(ctx context.Context, t *wo destShard := t.Attributes["destination_shard"] excludeTables := strings.Split(t.Attributes["exclude_tables"], ",") return hw.wr.CopySchemaShardFromShard(ctx, nil /* tableArray*/, excludeTables /* excludeTableArray */, true, /*includeViews*/ - keyspace, sourceShard, keyspace, destShard, wrangler.DefaultWaitSlaveTimeout) + keyspace, sourceShard, keyspace, destShard, wrangler.DefaultWaitSlaveTimeout, false) } func (hw *horizontalReshardingWorkflow) runSplitClone(ctx context.Context, t *workflowpb.Task) error { diff --git a/go/vt/workflow/resharding/workflow_test.go b/go/vt/workflow/resharding/workflow_test.go index 5224804db09..e13df204896 100644 --- a/go/vt/workflow/resharding/workflow_test.go +++ b/go/vt/workflow/resharding/workflow_test.go @@ -197,7 +197,7 @@ func splitCloneCommand(keyspace string, useConsistentSnapshot bool, excludeTable return args } -func splitDiffCommand(keyspace string, shardId string, useConsistentSnapshot bool, excludeTables, splitDiffCommand string) []string { +func splitDiffCommand(keyspace string, shardID string, useConsistentSnapshot bool, excludeTables, splitDiffCommand string) []string { args := []string{splitDiffCommand} if useConsistentSnapshot { args = append(args, "--use_consistent_snapshot") @@ -208,9 +208,9 @@ func splitDiffCommand(keyspace string, shardId string, useConsistentSnapshot boo switch splitDiffCommand { case "SplitDiff": - args = append(args, "--min_healthy_rdonly_tablets=1", "--dest_tablet_type=RDONLY", keyspace+"/"+shardId) + args = append(args, "--min_healthy_rdonly_tablets=1", "--dest_tablet_type=RDONLY", keyspace+"/"+shardID) case "MultiSplitDiff": - args = append(args, "--min_healthy_tablets=1", "--tablet_type=RDONLY", keyspace+"/"+shardId) + args = append(args, "--min_healthy_tablets=1", "--tablet_type=RDONLY", keyspace+"/"+shardID) } return args @@ -219,8 +219,8 @@ func splitDiffCommand(keyspace string, shardId string, useConsistentSnapshot boo func setupMockWrangler(ctrl *gomock.Controller, keyspace string) *MockReshardingWrangler { mockWranglerInterface := NewMockReshardingWrangler(ctrl) // Set the expected behaviors for mock wrangler. - mockWranglerInterface.EXPECT().CopySchemaShardFromShard(gomock.Any(), nil /* tableArray*/, gomock.Any() /* excludeTableArray */, true /*includeViews*/, keyspace, "0", keyspace, "-80", wrangler.DefaultWaitSlaveTimeout).Return(nil) - mockWranglerInterface.EXPECT().CopySchemaShardFromShard(gomock.Any(), nil /* tableArray*/, gomock.Any() /* excludeTableArray */, true /*includeViews*/, keyspace, "0", keyspace, "80-", wrangler.DefaultWaitSlaveTimeout).Return(nil) + mockWranglerInterface.EXPECT().CopySchemaShardFromShard(gomock.Any(), nil /* tableArray*/, gomock.Any() /* excludeTableArray */, true /*includeViews*/, keyspace, "0", keyspace, "-80", wrangler.DefaultWaitSlaveTimeout, false).Return(nil) + mockWranglerInterface.EXPECT().CopySchemaShardFromShard(gomock.Any(), nil /* tableArray*/, gomock.Any() /* excludeTableArray */, true /*includeViews*/, keyspace, "0", keyspace, "80-", wrangler.DefaultWaitSlaveTimeout, false).Return(nil) mockWranglerInterface.EXPECT().WaitForFilteredReplication(gomock.Any(), keyspace, "-80", wrangler.DefaultWaitForFilteredReplicationMaxDelay).Return(nil) mockWranglerInterface.EXPECT().WaitForFilteredReplication(gomock.Any(), keyspace, "80-", wrangler.DefaultWaitForFilteredReplicationMaxDelay).Return(nil) diff --git a/go/vt/wrangler/resharder.go b/go/vt/wrangler/resharder.go index f7199daa5e9..fec719d55ad 100644 --- a/go/vt/wrangler/resharder.go +++ b/go/vt/wrangler/resharder.go @@ -260,7 +260,7 @@ func (rs *resharder) identifyRuleType(rule *binlogdatapb.Rule) (int, error) { func (rs *resharder) copySchema(ctx context.Context) error { oneSource := rs.sourceShards[0].MasterAlias err := rs.forAll(rs.targetShards, func(target *topo.ShardInfo) error { - return rs.wr.CopySchemaShard(ctx, oneSource, []string{"/.*"}, nil, false, rs.keyspace, target.ShardName(), 1*time.Second) + return rs.wr.CopySchemaShard(ctx, oneSource, []string{"/.*"}, nil, false, rs.keyspace, target.ShardName(), 1*time.Second, false) }) return err } diff --git a/go/vt/wrangler/schema.go b/go/vt/wrangler/schema.go index 9044da86bb6..8e622b4777f 100644 --- a/go/vt/wrangler/schema.go +++ b/go/vt/wrangler/schema.go @@ -278,7 +278,7 @@ func (wr *Wrangler) PreflightSchema(ctx context.Context, tabletAlias *topodatapb // CopySchemaShardFromShard copies the schema from a source shard to the specified destination shard. // For both source and destination it picks the master tablet. See also CopySchemaShard. -func (wr *Wrangler) CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration) error { +func (wr *Wrangler) CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration, skipVerify bool) error { sourceShardInfo, err := wr.ts.GetShard(ctx, sourceKeyspace, sourceShard) if err != nil { return fmt.Errorf("GetShard(%v, %v) failed: %v", sourceKeyspace, sourceShard, err) @@ -287,14 +287,14 @@ func (wr *Wrangler) CopySchemaShardFromShard(ctx context.Context, tables, exclud return fmt.Errorf("no master in shard record %v/%v. Consider running 'vtctl InitShardMaster' in case of a new shard or to reparent the shard to fix the topology data, or providing a non-master tablet alias", sourceKeyspace, sourceShard) } - return wr.CopySchemaShard(ctx, sourceShardInfo.MasterAlias, tables, excludeTables, includeViews, destKeyspace, destShard, waitSlaveTimeout) + return wr.CopySchemaShard(ctx, sourceShardInfo.MasterAlias, tables, excludeTables, includeViews, destKeyspace, destShard, waitSlaveTimeout, skipVerify) } // CopySchemaShard copies the schema from a source tablet to the // specified shard. The schema is applied directly on the master of // the destination shard, and is propogated to the replicas through // binlogs. -func (wr *Wrangler) CopySchemaShard(ctx context.Context, sourceTabletAlias *topodatapb.TabletAlias, tables, excludeTables []string, includeViews bool, destKeyspace, destShard string, waitSlaveTimeout time.Duration) error { +func (wr *Wrangler) CopySchemaShard(ctx context.Context, sourceTabletAlias *topodatapb.TabletAlias, tables, excludeTables []string, includeViews bool, destKeyspace, destShard string, waitSlaveTimeout time.Duration, skipVerify bool) error { destShardInfo, err := wr.ts.GetShard(ctx, destKeyspace, destShard) if err != nil { return fmt.Errorf("GetShard(%v, %v) failed: %v", destKeyspace, destShard, err) @@ -349,12 +349,14 @@ func (wr *Wrangler) CopySchemaShard(ctx context.Context, sourceTabletAlias *topo // In that case, MySQL would have skipped our CREATE DATABASE IF NOT EXISTS // statement. We want to fail early in this case because vtworker SplitDiff // fails in case of such an inconsistency as well. - diffs, err = wr.compareSchemas(ctx, sourceTabletAlias, destShardInfo.MasterAlias, tables, excludeTables, includeViews) - if err != nil { - return fmt.Errorf("CopySchemaShard failed because schemas could not be compared finally: %v", err) - } - if diffs != nil { - return fmt.Errorf("CopySchemaShard was not successful because the schemas between the two tablets %v and %v differ: %v", sourceTabletAlias, destShardInfo.MasterAlias, diffs) + if !skipVerify { + diffs, err = wr.compareSchemas(ctx, sourceTabletAlias, destShardInfo.MasterAlias, tables, excludeTables, includeViews) + if err != nil { + return fmt.Errorf("CopySchemaShard failed because schemas could not be compared finally: %v", err) + } + if diffs != nil { + return fmt.Errorf("CopySchemaShard was not successful because the schemas between the two tablets %v and %v differ: %v", sourceTabletAlias, destShardInfo.MasterAlias, diffs) + } } // Notify slaves to reload schema. This is best-effort. From 492b7331193681c98038273c2bd9d203a0e75f21 Mon Sep 17 00:00:00 2001 From: Jacques Grove Date: Tue, 24 Mar 2020 21:15:48 -0700 Subject: [PATCH 2/2] Fix test. Signed-off-by: Jacques Grove --- go/vt/workflow/resharding/mock_resharding_wrangler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/workflow/resharding/mock_resharding_wrangler_test.go b/go/vt/workflow/resharding/mock_resharding_wrangler_test.go index 3b7ea4e23ba..c7ba1f8adc4 100644 --- a/go/vt/workflow/resharding/mock_resharding_wrangler_test.go +++ b/go/vt/workflow/resharding/mock_resharding_wrangler_test.go @@ -38,7 +38,7 @@ func (m *MockReshardingWrangler) EXPECT() *MockReshardingWranglerMockRecorder { // CopySchemaShardFromShard mocks base method func (m *MockReshardingWrangler) CopySchemaShardFromShard(ctx context.Context, tables, excludeTables []string, includeViews bool, sourceKeyspace, sourceShard, destKeyspace, destShard string, waitSlaveTimeout time.Duration, skipVerify bool) error { - ret := m.ctrl.Call(m, "CopySchemaShardFromShard", ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout) + ret := m.ctrl.Call(m, "CopySchemaShardFromShard", ctx, tables, excludeTables, includeViews, sourceKeyspace, sourceShard, destKeyspace, destShard, waitSlaveTimeout, false) ret0, _ := ret[0].(error) return ret0 }