Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VReplication: Properly ignore errors from trying to drop tables that don't exist #16505

Merged
merged 10 commits into from
Aug 8, 2024
3 changes: 2 additions & 1 deletion go/test/endtoend/vreplication/materialize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ const smMaterializeSchemaSource = `

const smMaterializeVSchemaSource = `
{
"sharded": true,
"tables": {
"mat": {
"column_vindexes": [
Expand Down Expand Up @@ -197,6 +196,8 @@ func testMaterialize(t *testing.T, useVtctldClient bool) {
_, err = ks2Primary.QueryTablet(customFunc, targetKs, true)
require.NoError(t, err)

testMaterializeWithNonExistentTable(t)

materialize(t, smMaterializeSpec2, useVtctldClient)
catchup(t, ks2Primary, "wf1", "Materialize")

Expand Down
12 changes: 12 additions & 0 deletions go/test/endtoend/vreplication/vreplication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,18 @@ func materialize(t *testing.T, spec string, useVtctldClient bool) {
}
}

func testMaterializeWithNonExistentTable(t *testing.T) {
t.Run("vtctldclient materialize with nonexistent table", func(t *testing.T) {
tableSettings := `[{"target_table": "table_that_doesnt_exist", "create_ddl": "create table mat_val_counts (mat_val varbinary(10), cnt int unsigned, primary key (mat_val))", "source_expression": "select val, count(*) as cnt from mat group by val"}]`
output, err := vc.VtctldClient.ExecuteCommandWithOutput("materialize", "--workflow=tablenogood", "--target-keyspace=source",
"create", "--source-keyspace=source", "--table-settings", tableSettings)
require.NoError(t, err, "Materialize create failed, err: %v, output: %s", err, output)
waitForWorkflowState(t, vc, "source.tablenogood", binlogdatapb.VReplicationWorkflowState_Stopped.String())
output, err = vc.VtctldClient.ExecuteCommandWithOutput("materialize", "--workflow=tablenogood", "--target-keyspace=source", "cancel")
require.NoError(t, err, "Materialize cancel failed, err: %v, output: %s", err, output)
})
}

func materializeProduct(t *testing.T, useVtctldClient bool) {
t.Run("materializeProduct", func(t *testing.T) {
// Materializing from "product" keyspace to "customer" keyspace.
Expand Down
3 changes: 1 addition & 2 deletions go/vt/vtctl/workflow/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/proto"

"vitess.io/vitess/go/mysql/sqlerror"
"vitess.io/vitess/go/protoutil"
"vitess.io/vitess/go/sets"
"vitess.io/vitess/go/sqlescape"
Expand Down Expand Up @@ -2548,7 +2547,7 @@ func (s *Server) optimizeCopyStateTable(tablet *topodatapb.Tablet) {
Query: []byte(sqlOptimizeTable),
MaxRows: uint64(100), // always produces 1+rows with notes and status
}); err != nil {
if sqlErr, ok := err.(*sqlerror.SQLError); ok && sqlErr.Num == sqlerror.ERNoSuchTable { // the table may not exist
if IsTableDidNotExistError(err) {
return
}
log.Warningf("Failed to optimize the copy_state table on %q: %v", tablet.Alias.String(), err)
Expand Down
17 changes: 8 additions & 9 deletions go/vt/vtctl/workflow/traffic_switcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"golang.org/x/sync/errgroup"

"vitess.io/vitess/go/json2"
"vitess.io/vitess/go/mysql/sqlerror"
"vitess.io/vitess/go/sqlescape"
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/binlog/binlogplayer"
Expand Down Expand Up @@ -548,12 +547,12 @@ func (ts *trafficSwitcher) removeSourceTables(ctx context.Context, removalType T
DisableForeignKeyChecks: true,
})
if err != nil {
if mysqlErr, ok := err.(*sqlerror.SQLError); ok && mysqlErr.Num == sqlerror.ERNoSuchTable {
if IsTableDidNotExistError(err) {
ts.Logger().Warningf("%s: Table %s did not exist when attempting to remove it", topoproto.TabletAliasString(source.GetPrimary().GetAlias()), tableName)
return nil
Copy link
Contributor Author

@mattlord mattlord Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a separate bug fixed here that would have caused the Delete/Cancel to leave any yet to be removed tables orphaned on the source/target. We never got into this block due to the noted issues, however, so this bug could not be triggered before this PR and thus in effect did not exist. The unit test (and framework used) was also updated to test for this.

} else {
ts.Logger().Errorf("%s: Error removing table %s: %v", topoproto.TabletAliasString(source.GetPrimary().GetAlias()), tableName, err)
return err
}
ts.Logger().Errorf("%s: Error removing table %s: %v", topoproto.TabletAliasString(source.GetPrimary().GetAlias()), tableName, err)
return err
}
ts.Logger().Infof("%s: Removed table %s.%s\n", topoproto.TabletAliasString(source.GetPrimary().GetAlias()), source.GetPrimary().DbName(), tableName)

Expand Down Expand Up @@ -1179,13 +1178,13 @@ func (ts *trafficSwitcher) removeTargetTables(ctx context.Context) error {
})
log.Infof("Removed target table with result: %+v", res)
if err != nil {
if mysqlErr, ok := err.(*sqlerror.SQLError); ok && mysqlErr.Num == sqlerror.ERNoSuchTable {
if IsTableDidNotExistError(err) {
// The table was already gone, so we can ignore the error.
ts.Logger().Warningf("%s: Table %s did not exist when attempting to remove it", topoproto.TabletAliasString(target.GetPrimary().GetAlias()), tableName)
return nil
} else {
ts.Logger().Errorf("%s: Error removing table %s: %v", topoproto.TabletAliasString(target.GetPrimary().GetAlias()), tableName, err)
return err
}
ts.Logger().Errorf("%s: Error removing table %s: %v", topoproto.TabletAliasString(target.GetPrimary().GetAlias()), tableName, err)
return err
}
ts.Logger().Infof("%s: Removed table %s.%s\n",
topoproto.TabletAliasString(target.GetPrimary().GetAlias()), target.GetPrimary().DbName(), tableName)
Expand Down
12 changes: 12 additions & 0 deletions go/vt/vtctl/workflow/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"strings"
"sync"

"vitess.io/vitess/go/mysql/sqlerror"
querypb "vitess.io/vitess/go/vt/proto/query"

"vitess.io/vitess/go/vt/vtgate/vindexes"
Expand Down Expand Up @@ -949,3 +950,14 @@ func getTabletTypeSuffix(tabletType topodatapb.TabletType) string {
}
return ""
}

// IsTableDidNotExistError will convert the given error to an sqlerror.SQLError and if
// the error code is ERNoSuchTable or ERBadTable, it will return true. This is helpful
// when e.g. processing a gRPC error which will be a status.Error that needs to be
// converted to an sqlerror.SQLError before we can examine the error code.
func IsTableDidNotExistError(err error) bool {
if mysqlErr, ok := sqlerror.NewSQLErrorFromError(err).(*sqlerror.SQLError); ok {
return mysqlErr.Num == sqlerror.ERNoSuchTable || mysqlErr.Num == sqlerror.ERBadTable
}
return false
}
Loading