-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Mysqld.GetSchema: tolerate tables being dropped while inspecting schema #12641
Changes from 2 commits
be883bc
eed2706
85f0273
79cfdc2
6ebebd8
39cdc58
6d866c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ import ( | |
"strings" | ||
"sync" | ||
|
||
"vitess.io/vitess/go/mysql" | ||
"vitess.io/vitess/go/sqltypes" | ||
"vitess.io/vitess/go/vt/concurrency" | ||
"vitess.io/vitess/go/vt/vterrors" | ||
|
@@ -109,6 +110,16 @@ func (mysqld *Mysqld) GetSchema(ctx context.Context, dbName string, request *tab | |
|
||
fields, columns, schema, err := mysqld.collectSchema(ctx, dbName, td.Name, td.Type, request.TableSchemaOnly) | ||
if err != nil { | ||
// there's a possible race condition: it could happen that a table was dropped in between reading | ||
// the list of tables (collectBasicTableData(), earlier) and the point above where we investigate | ||
// the table. | ||
// This is fine. We identify the situation and continue to remove the table from our records | ||
mattlord marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sqlErr, isSQLErr := mysql.NewSQLErrorFromError(err).(*mysql.SQLError) | ||
if isSQLErr && sqlErr != nil && sqlErr.Number() == mysql.ERNoSuchTable { | ||
td.Fields = nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we instead remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I rewrote this in a different way. Problem is that we're iterating |
||
return | ||
} | ||
|
||
allErrors.RecordError(err) | ||
cancel() | ||
return | ||
|
@@ -121,6 +132,8 @@ func (mysqld *Mysqld) GetSchema(ctx context.Context, dbName string, request *tab | |
} | ||
|
||
// Get primary columns concurrently. | ||
// The below runs a single query on `INFORMATION_SCHEMA` and does not interact with the actual tables. | ||
// It is therefore safe to run even if some tables are dropped in the interim. | ||
colMap := map[string][]string{} | ||
if len(tableNames) > 0 { | ||
wg.Add(1) | ||
|
@@ -146,7 +159,14 @@ func (mysqld *Mysqld) GetSchema(ctx context.Context, dbName string, request *tab | |
td.PrimaryKeyColumns = colMap[td.Name] | ||
} | ||
|
||
sd.TableDefinitions = tds | ||
sd.TableDefinitions = []*tabletmanagerdatapb.TableDefinition{} | ||
for _, td := range tds { | ||
if td.Fields == nil { | ||
// means table was dropped before we called GetColumns | ||
continue | ||
} | ||
mattlord marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sd.TableDefinitions = append(sd.TableDefinitions, td) | ||
} | ||
|
||
tmutils.GenerateSchemaVersion(sd) | ||
return sd, nil | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IF you have to do another push for some reason, we can capitalize
There's
here. It's annoyingly nitty, I know, so no need unless we have to do another commit/push for another reason.