VReplication: ignore GC tables in schema analysis (#12320)

* TableFilter: fix excluded RE listing Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * extract GCTableNameExpression Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * VReplication: exclude GC tables when reading schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * wrap schema analyzis errors for better tracability Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
vitessio · Feb 14, 2023 · 721d581 · 721d581
1 parent 1176327
commit 721d581
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 8 deletions.
diff --git a/go/vt/mysqlctl/schema.go b/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))

diff --git a/go/vt/schema/name_test.go b/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",

diff --git a/go/vt/schema/tablegc.go b/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,

diff --git a/go/vt/vttablet/onlineddl/analysis.go b/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:

diff --git a/go/vt/vttablet/tabletmanager/vreplication/vreplicator.go b/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

diff --git a/go/vt/vttablet/tabletserver/schema/engine.go b/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 {