VReplication: ignore GC tables in schema analysis

go/vt/mysqlctl/schema.go

@@ -232,7 +232,7 @@ func (mysqld *Mysqld) normalizedSchema(ctx context.Context, dbName, tableName, t
 	backtickDBName := sqlescape.EscapeID(dbName)
 	qr, fetchErr := mysqld.FetchSuperQuery(ctx, fmt.Sprintf("SHOW CREATE TABLE %s.%s", backtickDBName, sqlescape.EscapeID(tableName)))
 	if fetchErr != nil {
-		return "", fetchErr
+		return "", vterrors.Wrapf(fetchErr, "in Mysqld.normalizedSchema()")
 	}
 	if len(qr.Rows) == 0 {
 		return "", fmt.Errorf("empty create table statement for %v", tableName)
@@ -322,7 +322,7 @@ func GetColumns(dbName, table string, exec func(string, int, bool) (*sqltypes.Re
 	query := fmt.Sprintf(GetFieldsQuery, selectColumns, tableSpec)
 	qr, err := exec(query, 0, true)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, vterrors.Wrapf(err, "in Mysqld.GetColumns()")
 	}
 
 	columns := make([]string, len(qr.Fields))

go/vt/mysqlctl/tmutils/schema.go

@@ -119,7 +119,7 @@ func NewTableFilter(tables, excludeTables []string, includeViews bool) (*TableFi
 					return nil, fmt.Errorf("cannot compile regexp %v for excludeTable: %v", table, err)
 				}
 
-				f.excludeTableREs = append(f.tableREs, re)
+				f.excludeTableREs = append(f.excludeTableREs, re)
 			} else {
 				f.excludeTableNames = append(f.excludeTableNames, table)
 			}

go/vt/mysqlctl/tmutils/schema_test.go

@@ -19,9 +19,10 @@ package tmutils
 import (
 	"errors"
 	"fmt"
-	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
 
 	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
@@ -135,9 +136,7 @@ func TestToSQLStrings(t *testing.T) {
 
 	for _, tc := range testcases {
 		got := SchemaDefinitionToSQLStrings(tc.input)
-		if !reflect.DeepEqual(got, tc.want) {
-			t.Errorf("ToSQLStrings() on SchemaDefinition %v returned %v; want %v", tc.input, got, tc.want)
-		}
+		assert.Equal(t, tc.want, got)
 	}
 }
 
@@ -156,12 +155,7 @@ func testDiff(t *testing.T, left, right *tabletmanagerdatapb.SchemaDefinition, l
 			}
 		}
 	}
-
-	if !equal {
-		t.Logf("Expected: %v", expected)
-		t.Logf("Actual: %v", actual)
-		t.Fail()
-	}
+	assert.Truef(t, equal, "expected: %v, actual: %v", expected, actual)
 }
 
 func TestSchemaDiff(t *testing.T) {
@@ -433,6 +427,26 @@ func TestTableFilter(t *testing.T) {
 
 			included: false,
 		},
+		{
+			desc:          "exclude table list does not list table",
+			excludeTables: []string{"nomatch1", "nomatch2", "/nomatch3/", "/nomatch4/", "/nomatch5/"},
+			includeViews:  true,
+
+			tableName: excludedTable,
+			tableType: TableBaseTable,
+
+			included: true,
+		},
+		{
+			desc:          "exclude table list with re match",
+			excludeTables: []string{"nomatch1", "nomatch2", "/nomatch3/", "/" + excludedTable + "/", "/nomatch5/"},
+			includeViews:  true,
+
+			tableName: excludedTable,
+			tableType: TableBaseTable,
+
+			included: false,
+		},
 		{
 			desc:   "bad table regexp",
 			tables: []string{"/*/"},
@@ -450,18 +464,16 @@ func TestTableFilter(t *testing.T) {
 	for _, tc := range tcs {
 		t.Run(tc.desc, func(t *testing.T) {
 			f, err := NewTableFilter(tc.tables, tc.excludeTables, tc.includeViews)
-			if tc.hasErr != (err != nil) {
-				t.Fatalf("hasErr not right: %v, tc: %+v", err, tc)
-			}
-
 			if tc.hasErr {
+				assert.Error(t, err)
 				return
 			}
+			assert.NoError(t, err)
 
+			assert.Equal(t, len(tc.tables), len(f.tableNames)+len(f.tableREs))
+			assert.Equal(t, len(tc.excludeTables), len(f.excludeTableNames)+len(f.excludeTableREs))
 			included := f.Includes(tc.tableName, tc.tableType)
-			if tc.included != included {
-				t.Fatalf("included is not right: %v\nfilter: %+v\ntc: %+v", included, f, tc)
-			}
+			assert.Equalf(t, tc.included, included, "filter: %v", f)
 		})
 	}
 }
@@ -638,21 +650,15 @@ func TestFilterTables(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		got, err := FilterTables(tc.input, tc.tables, tc.excludeTables, tc.includeViews)
-		if tc.wantError != nil {
-			if err == nil {
-				t.Fatalf("FilterTables() test '%v' on SchemaDefinition %v did not return an error (result: %v), but should have, wantError %v", tc.desc, tc.input, got, tc.wantError)
-			}
-			if err.Error() != tc.wantError.Error() {
-				t.Errorf("FilterTables() test '%v' on SchemaDefinition %v returned wrong error '%v'; wanted error '%v'", tc.desc, tc.input, err, tc.wantError)
-			}
-		} else {
-			if err != nil {
-				t.Errorf("FilterTables() test '%v' on SchemaDefinition %v failed with error %v, want %v", tc.desc, tc.input, err, tc.want)
-			}
-			if !proto.Equal(got, tc.want) {
-				t.Errorf("FilterTables() test '%v' on SchemaDefinition %v returned %v; want %v", tc.desc, tc.input, got, tc.want)
+		t.Run(tc.desc, func(t *testing.T) {
+			got, err := FilterTables(tc.input, tc.tables, tc.excludeTables, tc.includeViews)
+			if tc.wantError != nil {
+				require.Error(t, err)
+				require.Equal(t, tc.wantError, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Truef(t, proto.Equal(tc.want, got), "wanted: %v, got: %v", tc.want, got)
 			}
-		}
+		})
 	}
 }

go/vt/schema/name_test.go

@@ -22,6 +22,41 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestNameIsGCTableName(t *testing.T) {
+	irrelevantNames := []string{
+		"t",
+		"_table_new",
+		"__table_new",
+		"_table_gho",
+		"_table_ghc",
+		"_table_del",
+		"table_old",
+		"vt_onlineddl_test_02",
+		"_4e5dcf80_354b_11eb_82cd_f875a4d24e90_20201203114014_gho",
+		"_4e5dcf80_354b_11eb_82cd_f875a4d24e90_20201203114014_ghc",
+		"_4e5dcf80_354b_11eb_82cd_f875a4d24e90_20201203114014_del",
+		"_4e5dcf80_354b_11eb_82cd_f875a4d24e90_20201203114013_new",
+		"_table_old",
+		"__table_old",
+	}
+	for _, tableName := range irrelevantNames {
+		t.Run(tableName, func(t *testing.T) {
+			assert.False(t, IsGCTableName(tableName))
+		})
+	}
+	relevantNames := []string{
+		"_vt_DROP_6ace8bcef73211ea87e9f875a4d24e90_20200915120410",
+		"_vt_HOLD_6ace8bcef73211ea87e9f875a4d24e90_20200915120410",
+		"_vt_EVAC_6ace8bcef73211ea87e9f875a4d24e90_20200915120410",
+		"_vt_PURGE_6ace8bcef73211ea87e9f875a4d24e90_20200915120410",
+	}
+	for _, tableName := range relevantNames {
+		t.Run(tableName, func(t *testing.T) {
+			assert.True(t, IsGCTableName(tableName))
+		})
+	}
+}
+
 func TestIsInternalOperationTableName(t *testing.T) {
 	names := []string{
 		"_4e5dcf80_354b_11eb_82cd_f875a4d24e90_20201203114014_gho",

go/vt/schema/tablegc.go

@@ -44,9 +44,13 @@ const (
 	TableDroppedGCState TableGCState = ""
 )
 
+const (
+	GCTableNameExpression string = `^_vt_(HOLD|PURGE|EVAC|DROP)_([0-f]{32})_([0-9]{14})$`
+)
+
 var (
 	gcUUIDRegexp      = regexp.MustCompile(`^[0-f]{32}$`)
-	gcTableNameRegexp = regexp.MustCompile(`^_vt_(HOLD|PURGE|EVAC|DROP)_([0-f]{32})_([0-9]{14})$`)
+	gcTableNameRegexp = regexp.MustCompile(GCTableNameExpression)
 
 	gcStates = map[string]TableGCState{
 		string(HoldTableGCState):  HoldTableGCState,

go/vt/vttablet/onlineddl/analysis.go

@@ -73,7 +73,7 @@ func (p *SpecialAlterPlan) String() string {
 func (e *Executor) getCreateTableStatement(ctx context.Context, tableName string) (*sqlparser.CreateTable, error) {
 	showCreateTable, err := e.showCreateTable(ctx, tableName)
 	if err != nil {
-		return nil, err
+		return nil, vterrors.Wrapf(err, "in Executor.getCreateTableStatement()")
 	}
 	stmt, err := sqlparser.ParseStrictDDL(showCreateTable)
 	if err != nil {
@@ -360,7 +360,7 @@ func (e *Executor) analyzeSpecialAlterPlan(ctx context.Context, onlineDDL *schem
 
 	createTable, err := e.getCreateTableStatement(ctx, onlineDDL.Table)
 	if err != nil {
-		return nil, err
+		return nil, vterrors.Wrapf(err, "in Executor.analyzeSpecialAlterPlan(), uuid=%v, table=%v", onlineDDL.UUID, onlineDDL.Table)
 	}
 
 	// special plans which support reverts are trivially desired:

go/vt/vttablet/tabletmanager/vreplication/vreplicator.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"vitess.io/vitess/go/timer"
+	"vitess.io/vitess/go/vt/schema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
 
@@ -287,7 +288,7 @@ type ColumnInfo struct {
 }
 
 func (vr *vreplicator) buildColInfoMap(ctx context.Context) (map[string][]*ColumnInfo, error) {
-	req := &tabletmanagerdatapb.GetSchemaRequest{Tables: []string{"/.*/"}}
+	req := &tabletmanagerdatapb.GetSchemaRequest{Tables: []string{"/.*/"}, ExcludeTables: []string{"/" + schema.GCTableNameExpression + "/"}}
 	schema, err := vr.mysqld.GetSchema(ctx, vr.dbClient.DBName(), req)
 	if err != nil {
 		return nil, err

go/vt/vttablet/tabletserver/schema/engine.go

@@ -405,7 +405,7 @@ func (se *Engine) reload(ctx context.Context, includeStats bool) error {
 	}
 	tableData, err := conn.Exec(ctx, showTablesQuery, maxTableCount, false)
 	if err != nil {
-		return err
+		return vterrors.Wrapf(err, "in Engine.reload(), reading tables")
 	}
 
 	err = se.updateInnoDBRowsRead(ctx, conn)
@@ -456,7 +456,7 @@ func (se *Engine) reload(ctx context.Context, includeStats bool) error {
 		log.V(2).Infof("Reading schema for table: %s", tableName)
 		table, err := LoadTable(conn, se.cp.DBName(), tableName, row[3].ToString())
 		if err != nil {
-			rec.RecordError(err)
+			rec.RecordError(vterrors.Wrapf(err, "in Engine.reload(), reading table %s", tableName))
 			continue
 		}
 		if includeStats {