Skip to content

Commit bc747f5

Browse files
author
Clemens Kolbitsch
committed
Work around CREATE TABLE GCP bug
GCP cloud-SQL DBs do not seem to allow using gh-ost due to the way that we create tables (using `CREATE TABLE <ghost-table> LIKE <table>`). This is a confirmed GCP bug: https://issuetracker.google.com/issues/158129960 For the time being, this commit adds an option to enable GCP support, using a different CREATE TABLE statement.
1 parent 4dab06e commit bc747f5

File tree

9 files changed

+65
-19
lines changed

9 files changed

+65
-19
lines changed

RELEASE_VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.0.49
1+
1.0.49-0lastline1

doc/command-line-flags.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ Table name prefix to be used on the temporary tables.
127127

128128
Add this flag when executing on a 1st generation Google Cloud Platform (GCP).
129129

130+
### gcpv2
131+
132+
Add this flag when executing on a 2nd generation Google Cloud Platform (GCP).
133+
130134
### heartbeat-interval-millis
131135

132136
Default 100. See [`subsecond-lag`](subsecond-lag.md) for details.

doc/requirements-and-limitations.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ The `SUPER` privilege is required for `STOP SLAVE`, `START SLAVE` operations. Th
3939
- For example, you may not migrate `MyTable` if another table called `MYtable` exists in the same schema.
4040

4141
- Amazon RDS works, but has its own [limitations](rds.md).
42-
- Google Cloud SQL works, `--gcp` flag required.
42+
- Google Cloud SQL works, `--gcp` or `--gcpv2` flag required for running on a 1st/2nd generation Cloud SQL, respectively.
4343
- Aliyun RDS works, `--aliyun-rds` flag required.
4444

4545
- Multisource is not supported when migrating via replica. It _should_ work (but never tested) when connecting directly to master (`--allow-on-master`)

go/base/context.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ type MigrationContext struct {
9494
IsTungsten bool
9595
DiscardForeignKeys bool
9696
AliyunRDS bool
97-
GoogleCloudPlatform bool
97+
GoogleCloudPlatformV1 bool
98+
GoogleCloudPlatformV2 bool
9899

99100
config ContextConfig
100101
configMutex *sync.Mutex

go/base/utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig,
7676
}
7777
// AliyunRDS set users port to "NULL", replace it by gh-ost param
7878
// GCP set users port to "NULL", replace it by gh-ost param
79-
if migrationContext.AliyunRDS || migrationContext.GoogleCloudPlatform {
79+
if migrationContext.AliyunRDS || migrationContext.GoogleCloudPlatformV1 {
8080
port = connectionConfig.Key.Port
8181
} else {
8282
portQuery := `select @@global.port`

go/cmd/gh-ost/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ func main() {
7676
flag.BoolVar(&migrationContext.SkipForeignKeyChecks, "skip-foreign-key-checks", false, "set to 'true' when you know for certain there are no foreign keys on your table, and wish to skip the time it takes for gh-ost to verify that")
7777
flag.BoolVar(&migrationContext.SkipStrictMode, "skip-strict-mode", false, "explicitly tell gh-ost binlog applier not to enforce strict sql mode")
7878
flag.BoolVar(&migrationContext.AliyunRDS, "aliyun-rds", false, "set to 'true' when you execute on Aliyun RDS.")
79-
flag.BoolVar(&migrationContext.GoogleCloudPlatform, "gcp", false, "set to 'true' when you execute on a 1st generation Google Cloud Platform (GCP).")
79+
flag.BoolVar(&migrationContext.GoogleCloudPlatformV1, "gcp", false, "set to 'true' when you execute on a 1st generation Google Cloud Platform (GCP).")
80+
flag.BoolVar(&migrationContext.GoogleCloudPlatformV2, "gcpv2", false, "set to 'true' when you execute on a 2nd generation Google Cloud Platform (GCP).")
8081

8182
executeFlag := flag.Bool("execute", false, "actually execute the alter & migrate the table. Default is noop: do some tests and exit")
8283
flag.BoolVar(&migrationContext.TestOnReplica, "test-on-replica", false, "Have the migration run on a replica, not on the master. At the end of migration replication is stopped, and tables are swapped and immediately swap-revert. Replication remains stopped and you can compare the two tables for building trust")

go/logic/applier.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func (this *Applier) InitDBConnections() (err error) {
8989
if err := this.validateAndReadTimeZone(); err != nil {
9090
return err
9191
}
92-
if !this.migrationContext.AliyunRDS && !this.migrationContext.GoogleCloudPlatform {
92+
if !this.migrationContext.AliyunRDS && !this.migrationContext.GoogleCloudPlatformV1 {
9393
if impliedKey, err := mysql.GetInstanceKey(this.db); err != nil {
9494
return err
9595
} else {
@@ -168,18 +168,12 @@ func (this *Applier) ValidateOrDropExistingTables() error {
168168
}
169169

170170
// CreateGhostTable creates the ghost table on the applier host
171-
func (this *Applier) CreateGhostTable() error {
172-
query := fmt.Sprintf(`create /* gh-ost */ table %s.%s like %s.%s`,
173-
sql.EscapeName(this.migrationContext.DatabaseName),
174-
sql.EscapeName(this.migrationContext.GetGhostTableName()),
175-
sql.EscapeName(this.migrationContext.DatabaseName),
176-
sql.EscapeName(this.migrationContext.OriginalTableName),
177-
)
178-
log.Infof("Creating ghost table %s.%s",
171+
func (this *Applier) CreateGhostTable(createTableStatement string) error {
172+
log.Infof("Creating ghost table `%s`.`%s`",
179173
sql.EscapeName(this.migrationContext.DatabaseName),
180174
sql.EscapeName(this.migrationContext.GetGhostTableName()),
181175
)
182-
if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
176+
if _, err := sqlutils.ExecNoPrepare(this.db, createTableStatement); err != nil {
183177
return err
184178
}
185179
log.Infof("Ghost table created")

go/logic/inspect.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (this *Inspector) InitDBConnections() (err error) {
5353
if err := this.validateConnection(); err != nil {
5454
return err
5555
}
56-
if !this.migrationContext.AliyunRDS && !this.migrationContext.GoogleCloudPlatform {
56+
if !this.migrationContext.AliyunRDS && !this.migrationContext.GoogleCloudPlatformV1 {
5757
if impliedKey, err := mysql.GetInstanceKey(this.db); err != nil {
5858
return err
5959
} else {
@@ -729,7 +729,7 @@ func (this *Inspector) getSharedColumns(originalColumns, ghostColumns *sql.Colum
729729
}
730730

731731
// showCreateTable returns the `show create table` statement for given table
732-
func (this *Inspector) showCreateTable(tableName string) (createTableStatement string, err error) {
732+
func (this *Inspector) ShowCreateTable(tableName string) (createTableStatement string, err error) {
733733
var dummy string
734734
query := fmt.Sprintf(`show /* gh-ost */ create table %s.%s`, sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(tableName))
735735
err = this.db.QueryRow(query).Scan(&dummy, &createTableStatement)

go/logic/migrator.go

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,46 @@ func (this *Migrator) initiateThrottler() error {
10481048
return nil
10491049
}
10501050

1051+
func (this *Migrator) getCreateGhostTableStatement() (string, error) {
1052+
fullOriginalTableName := fmt.Sprintf(`%s.%s`,
1053+
sql.EscapeName(this.migrationContext.DatabaseName),
1054+
sql.EscapeName(this.migrationContext.OriginalTableName),
1055+
)
1056+
fullGhostTableName := fmt.Sprintf(`%s.%s`,
1057+
sql.EscapeName(this.migrationContext.DatabaseName),
1058+
sql.EscapeName(this.migrationContext.GetGhostTableName()),
1059+
)
1060+
1061+
var createGhostTableStatement string
1062+
// XXX: It would be better to have a unified way of creating tables - is there a downside to
1063+
// using the rename? The replace strategy seems a bit "simple", but it is how other tools do
1064+
// it too (e.g., ghostferry)
1065+
if this.migrationContext.GoogleCloudPlatformV2 {
1066+
createTableStatement, err := this.inspector.ShowCreateTable(this.migrationContext.OriginalTableName)
1067+
if err != nil {
1068+
return "", err
1069+
}
1070+
1071+
createGhostTableStatement = strings.Replace(
1072+
createTableStatement,
1073+
// NOTE: MySQL always gives a CREATE TABLE using the quotes table name without
1074+
// the database name
1075+
fmt.Sprintf("CREATE TABLE %s", sql.EscapeName(this.migrationContext.OriginalTableName)),
1076+
fmt.Sprintf("CREATE TABLE %s", fullGhostTableName),
1077+
1,
1078+
)
1079+
log.Debugf("Using GCP-compatible create statement: %s", createGhostTableStatement)
1080+
} else {
1081+
createGhostTableStatement = fmt.Sprintf(`create /* gh-ost */ table %s like %s`,
1082+
fullGhostTableName,
1083+
fullOriginalTableName,
1084+
)
1085+
log.Debugf("Using default create statement: %s", createGhostTableStatement)
1086+
}
1087+
1088+
return createGhostTableStatement, nil
1089+
}
1090+
10511091
func (this *Migrator) initiateApplier() error {
10521092
this.applier = NewApplier(this.migrationContext)
10531093
if err := this.applier.InitDBConnections(); err != nil {
@@ -1060,7 +1100,13 @@ func (this *Migrator) initiateApplier() error {
10601100
log.Errorf("Unable to create changelog table, see further error details. Perhaps a previous migration failed without dropping the table? OR is there a running migration? Bailing out")
10611101
return err
10621102
}
1063-
if err := this.applier.CreateGhostTable(); err != nil {
1103+
1104+
createTableStatement, err := this.getCreateGhostTableStatement()
1105+
if err != nil {
1106+
log.Errorf("Unable to get ghost table schema, see further error details.")
1107+
return err
1108+
}
1109+
if err := this.applier.CreateGhostTable(createTableStatement); err != nil {
10641110
log.Errorf("Unable to create ghost table, see further error details. Perhaps a previous migration failed without dropping the table? Bailing out")
10651111
return err
10661112
}
@@ -1262,7 +1308,7 @@ func (this *Migrator) finalCleanup() error {
12621308
atomic.StoreInt64(&this.migrationContext.CleanupImminentFlag, 1)
12631309

12641310
if this.migrationContext.Noop {
1265-
if createTableStatement, err := this.inspector.showCreateTable(this.migrationContext.GetGhostTableName()); err == nil {
1311+
if createTableStatement, err := this.inspector.ShowCreateTable(this.migrationContext.GetGhostTableName()); err == nil {
12661312
log.Infof("New table structure follows")
12671313
fmt.Println(createTableStatement)
12681314
} else {

0 commit comments

Comments
 (0)