From 2793e2b6b317465e06a9e9a99f4af52ebd64a304 Mon Sep 17 00:00:00 2001 From: Hasan Mshawrab <63023909+hasanMshawrab@users.noreply.github.com> Date: Wed, 26 Oct 2022 10:28:27 +0300 Subject: [PATCH 1/9] Fix: Change table name table name is 'tbl' not 'tble' --- doc/shared-key.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/shared-key.md b/doc/shared-key.md index c7f24cc..3dfa39b 100644 --- a/doc/shared-key.md +++ b/doc/shared-key.md @@ -29,7 +29,7 @@ CREATE TABLE tbl ( (This is also the definition of the _ghost_ table, except that that table would be called `_tbl_gho`). -In this migration, the _before_ and _after_ versions contain the same unique not-null key (the PRIMARY KEY). To run this migration, `gh-ost` would iterate through the `tbl` table using the primary key, copy rows from `tbl` to the _ghost_ table `_tbl_gho` in primary key order, while also applying the binlog event writes from `tble` onto `_tbl_gho`. +In this migration, the _before_ and _after_ versions contain the same unique not-null key (the PRIMARY KEY). To run this migration, `gh-ost` would iterate through the `tbl` table using the primary key, copy rows from `tbl` to the _ghost_ table `_tbl_gho` in primary key order, while also applying the binlog event writes from `tbl` onto `_tbl_gho`. The applying of the binlog events is what requires the shared unique key. For example, an `UPDATE` statement to `tbl` translates to a `REPLACE` statement which `gh-ost` applies to `_tbl_gho`. A `REPLACE` statement expects to insert or replace an existing row based on its row's values and the table's unique key constraints. In particular, if inserting that row would result in a unique key violation (e.g., a row with that primary key already exists), it would _replace_ that existing row with the new values. From 1be6a4c0823d3ed22f58ff1ff77bc34eff7797a0 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Wed, 9 Nov 2022 20:11:49 -0700 Subject: [PATCH 2/9] Attempt instant DDL if supported --- go/base/context.go | 1 + go/cmd/gh-ost/main.go | 2 ++ go/logic/applier.go | 48 +++++++++++++++++++++++++++++++++++++++++++ go/logic/migrator.go | 25 ++++++++++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/go/base/context.go b/go/base/context.go index 270b7a0..6032a93 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -101,6 +101,7 @@ type MigrationContext struct { AliyunRDS bool GoogleCloudPlatform bool AzureMySQL bool + AttemptInstantDDL bool config ContextConfig configMutex *sync.Mutex diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index b99e70b..660c492 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -67,6 +67,8 @@ func main() { flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") flag.StringVar(&migrationContext.AlterStatement, "alter", "", "alter statement (mandatory)") + flag.BoolVar(&migrationContext.AttemptInstantDDL, "attempt-instant-ddl", true, "Attempt to use instant DDL for this migration first.") + flag.BoolVar(&migrationContext.CountTableRows, "exact-rowcount", false, "actually count table rows as opposed to estimate them (results in more accurate progress estimation)") flag.BoolVar(&migrationContext.ConcurrentCountTableRows, "concurrent-rowcount", true, "(with --exact-rowcount), when true (default): count rows after row-copy begins, concurrently, and adjust row estimate later on; when false: first count rows, then start row copy") flag.BoolVar(&migrationContext.AllowedRunningOnMaster, "allow-on-master", false, "allow this migration to run directly on master. Preferably it would run on a replica") diff --git a/go/logic/applier.go b/go/logic/applier.go index 50fd9bd..b9313a3 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -188,6 +188,54 @@ func (this *Applier) ValidateOrDropExistingTables() error { return nil } +// AttemptInstantDDL attempts to use instant DDL (from MySQL 8.0, and earlier in Aurora and some others.) +// to apply the ALTER statement immediately. If it errors, the original +// gh-ost algorithm can be used. However, if it's successful -- a lot +// of time can potentially be saved. Instant operations include: +// - Adding a column +// - Dropping a column +// - Dropping an index +// - Extending a varchar column +// It is safer to attempt the change than try and parse the DDL, since +// there might be specifics about the table which make it not possible to apply instantly. +func (this *Applier) AttemptInstantDDL() error { + + query := fmt.Sprintf(`ALTER /* gh-ost */ TABLE %s.%s %s, ALGORITHM=INSTANT`, + sql.EscapeName(this.migrationContext.DatabaseName), + sql.EscapeName(this.migrationContext.OriginalTableName), + this.migrationContext.AlterStatementOptions, + ) + this.migrationContext.Log.Infof("INSTANT DDL Query is: %s", query) + + err := func() error { + tx, err := this.db.Begin() + if err != nil { + return err + } + defer tx.Rollback() + + sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone) + sessionQuery = fmt.Sprintf("%s, %s", sessionQuery, this.generateSqlModeQuery()) + + if _, err := tx.Exec(sessionQuery); err != nil { + return err + } + if _, err := tx.Exec(query); err != nil { + this.migrationContext.Log.Infof("INSTANT DDL failed: %s", err) + return err + } + if err := tx.Commit(); err != nil { + // Neither SET SESSION nor ALTER are really transactional, so strictly speaking + // there's no need to commit; but let's do this the legit way anyway. + return err + } + return nil + }() + + return err + +} + // CreateGhostTable creates the ghost table on the applier host func (this *Applier) CreateGhostTable() error { query := fmt.Sprintf(`create /* gh-ost */ table %s.%s like %s.%s`, diff --git a/go/logic/migrator.go b/go/logic/migrator.go index b443d69..40415d0 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -351,12 +351,26 @@ func (this *Migrator) Migrate() (err error) { if err := this.initiateInspector(); err != nil { return err } + // In MySQL 8.0 (and possibly earlier) some DDL statements can be applied instantly. + // As just a metadata change. We can't detect this unless we attempt the statement + // (i.e. there is no explain for DDL). + if this.migrationContext.AttemptInstantDDL { + this.migrationContext.Log.Infof("Attempting to execute ALTER TABLE as INSTANT DDL") + if err := this.attemptInstantDDL(); err == nil { + this.migrationContext.Log.Infof("Success! Table %s.%s migrated instantly", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName)) + return nil + } else { + this.migrationContext.Log.Infof("INSTANT DDL failed, will proceed with original algorithm: %s", err) + } + } + if err := this.initiateStreaming(); err != nil { return err } if err := this.initiateApplier(); err != nil { return err } + if err := this.createFlagFiles(); err != nil { return err } @@ -734,6 +748,17 @@ func (this *Migrator) initiateServer() (err error) { return nil } +// attemptInstantDDL tries to apply the DDL statement directly to the table +// using a ALGORITHM=INSTANT assertion. If this fails, it will return an error, +// in which case the original algorithm should be used. +func (this *Migrator) attemptInstantDDL() (err error) { + this.applier = NewApplier(this.migrationContext) + if err := this.applier.InitDBConnections(); err != nil { + return err + } + return this.applier.AttemptInstantDDL() +} + // initiateInspector connects, validates and inspects the "inspector" server. // The "inspector" server is typically a replica; it is where we issue some // queries such as: From 05f32ebf297456ee759ced15aaa124acb6e7d207 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Thu, 10 Nov 2022 09:30:13 -0700 Subject: [PATCH 3/9] minor cleanup --- go/logic/applier.go | 32 +++----------------------------- go/logic/migrator.go | 1 - 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index b9313a3..abda2b0 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -199,41 +199,15 @@ func (this *Applier) ValidateOrDropExistingTables() error { // It is safer to attempt the change than try and parse the DDL, since // there might be specifics about the table which make it not possible to apply instantly. func (this *Applier) AttemptInstantDDL() error { - query := fmt.Sprintf(`ALTER /* gh-ost */ TABLE %s.%s %s, ALGORITHM=INSTANT`, sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName), this.migrationContext.AlterStatementOptions, ) - this.migrationContext.Log.Infof("INSTANT DDL Query is: %s", query) - - err := func() error { - tx, err := this.db.Begin() - if err != nil { - return err - } - defer tx.Rollback() - - sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone) - sessionQuery = fmt.Sprintf("%s, %s", sessionQuery, this.generateSqlModeQuery()) - - if _, err := tx.Exec(sessionQuery); err != nil { - return err - } - if _, err := tx.Exec(query); err != nil { - this.migrationContext.Log.Infof("INSTANT DDL failed: %s", err) - return err - } - if err := tx.Commit(); err != nil { - // Neither SET SESSION nor ALTER are really transactional, so strictly speaking - // there's no need to commit; but let's do this the legit way anyway. - return err - } - return nil - }() - + this.migrationContext.Log.Infof("INSTANT DDL query is: %s", query) + // We don't need a trx, because for instant DDL the SQL mode doesn't matter. + _, err := this.db.Exec(query) return err - } // CreateGhostTable creates the ghost table on the applier host diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 40415d0..25989f8 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -370,7 +370,6 @@ func (this *Migrator) Migrate() (err error) { if err := this.initiateApplier(); err != nil { return err } - if err := this.createFlagFiles(); err != nil { return err } From 75a346be93a7ab3b9097fe05ef2451adeadccbdd Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 14 Nov 2022 11:29:26 -0700 Subject: [PATCH 4/9] Add tests, incorporate feedback --- doc/command-line-flags.md | 16 +++++++++++ go/cmd/gh-ost/main.go | 2 +- go/logic/applier.go | 33 ++++++++++++++--------- go/logic/applier_test.go | 14 ++++++++++ go/logic/migrator.go | 9 +++---- localtests/attempt-instant-ddl/create.sql | 13 +++++++++ localtests/attempt-instant-ddl/extra_args | 1 + 7 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 localtests/attempt-instant-ddl/create.sql create mode 100644 localtests/attempt-instant-ddl/extra_args diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md index dc481d0..b496b04 100644 --- a/doc/command-line-flags.md +++ b/doc/command-line-flags.md @@ -45,6 +45,22 @@ If you happen to _know_ your servers use RBR (Row Based Replication, i.e. `binlo Skipping this step means `gh-ost` would not need the `SUPER` privilege in order to operate. You may want to use this on Amazon RDS. +### attempt-instant-ddl + +MySQL 8.0 support "instant DDL" for some operations. If an alter statement can be completed with instant DDL, only a metadata change is required internally, so MySQL will return _instantly_ (only requiring a metadata lock to complete). Instant operations include: + +- Adding a column +- Dropping a column +- Dropping an index +- Extending a varchar column +- Adding a virtual generated column + +It is not reliable to parse the `ALTER` statement to determine if it is instant or not. This is because the table might be in an older row format, or have some other incompatibility that is difficult to identify. + +The risks of attempting to instant DDL are relatively minor: Gh-ost may need to acquire a metadata lock at the start of the operation. This is not a problem for most scenarios, but it could be a problem for users that start the DDL during a period with long running transactions. + +gh-ost will automatically fallback to the normal DDL process if the attempt to use instant DDL is unsuccessful. + ### conf `--conf=/path/to/my.cnf`: file where credentials are specified. Should be in (or contain) the following format: diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 660c492..c00b206 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -67,7 +67,7 @@ func main() { flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") flag.StringVar(&migrationContext.AlterStatement, "alter", "", "alter statement (mandatory)") - flag.BoolVar(&migrationContext.AttemptInstantDDL, "attempt-instant-ddl", true, "Attempt to use instant DDL for this migration first.") + flag.BoolVar(&migrationContext.AttemptInstantDDL, "attempt-instant-ddl", false, "Attempt to use instant DDL for this migration first") flag.BoolVar(&migrationContext.CountTableRows, "exact-rowcount", false, "actually count table rows as opposed to estimate them (results in more accurate progress estimation)") flag.BoolVar(&migrationContext.ConcurrentCountTableRows, "concurrent-rowcount", true, "(with --exact-rowcount), when true (default): count rows after row-copy begins, concurrently, and adjust row estimate later on; when false: first count rows, then start row copy") diff --git a/go/logic/applier.go b/go/logic/applier.go index abda2b0..ad6368e 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -135,6 +135,16 @@ func (this *Applier) generateSqlModeQuery() string { return fmt.Sprintf("sql_mode = %s", sqlModeQuery) } +// generateInstantDDLQuery returns the SQL for this ALTER operation +// with an INSTANT assertion (requires MySQL 8.0+) +func (this *Applier) generateInstantDDLQuery() string { + return fmt.Sprintf(`ALTER /* gh-ost */ TABLE %s.%s %s, ALGORITHM=INSTANT`, + sql.EscapeName(this.migrationContext.DatabaseName), + sql.EscapeName(this.migrationContext.OriginalTableName), + this.migrationContext.AlterStatementOptions, + ) +} + // readTableColumns reads table columns on applier func (this *Applier) readTableColumns() (err error) { this.migrationContext.Log.Infof("Examining table structure on applier") @@ -188,22 +198,21 @@ func (this *Applier) ValidateOrDropExistingTables() error { return nil } -// AttemptInstantDDL attempts to use instant DDL (from MySQL 8.0, and earlier in Aurora and some others.) -// to apply the ALTER statement immediately. If it errors, the original -// gh-ost algorithm can be used. However, if it's successful -- a lot -// of time can potentially be saved. Instant operations include: +// AttemptInstantDDL attempts to use instant DDL (from MySQL 8.0, and earlier in Aurora and some others). +// If successful, the operation is only a meta-data change so a lot of time is saved! +// The risk of attempting to instant DDL when not supported is that a metadata lock may be acquired. +// This is minor, since gh-ost will eventually require a metadata lock anyway, but at the cut-over stage. +// Instant operations include: // - Adding a column // - Dropping a column // - Dropping an index -// - Extending a varchar column -// It is safer to attempt the change than try and parse the DDL, since -// there might be specifics about the table which make it not possible to apply instantly. +// - Extending a VARCHAR column +// - Adding a virtual generated column +// It is not reliable to parse the `alter` statement to determine if it is instant or not. +// This is because the table might be in an older row format, or have some other incompatibility +// that is difficult to identify. func (this *Applier) AttemptInstantDDL() error { - query := fmt.Sprintf(`ALTER /* gh-ost */ TABLE %s.%s %s, ALGORITHM=INSTANT`, - sql.EscapeName(this.migrationContext.DatabaseName), - sql.EscapeName(this.migrationContext.OriginalTableName), - this.migrationContext.AlterStatementOptions, - ) + query := this.generateInstantDDLQuery() this.migrationContext.Log.Infof("INSTANT DDL query is: %s", query) // We don't need a trx, because for instant DDL the SQL mode doesn't matter. _, err := this.db.Exec(query) diff --git a/go/logic/applier_test.go b/go/logic/applier_test.go index a2c1414..44d1fc7 100644 --- a/go/logic/applier_test.go +++ b/go/logic/applier_test.go @@ -170,3 +170,17 @@ func TestApplierBuildDMLEventQuery(t *testing.T) { test.S(t).ExpectEquals(res[0].args[3], 42) }) } + +func TestApplierInstantDDL(t *testing.T) { + migrationContext := base.NewMigrationContext() + migrationContext.DatabaseName = "test" + migrationContext.OriginalTableName = "mytable" + migrationContext.AlterStatementOptions = "ADD INDEX (foo)" + applier := NewApplier(migrationContext) + + t.Run("instantDDLstmt", func(t *testing.T) { + stmt := applier.generateInstantDDLQuery() + test.S(t).ExpectEquals(stmt, "ALTER /* gh-ost */ TABLE `test`.`mytable` ADD INDEX (foo), ALGORITHM=INSTANT") + }) + +} diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 25989f8..13cbdee 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -352,15 +352,14 @@ func (this *Migrator) Migrate() (err error) { return err } // In MySQL 8.0 (and possibly earlier) some DDL statements can be applied instantly. - // As just a metadata change. We can't detect this unless we attempt the statement - // (i.e. there is no explain for DDL). + // Attempt to do this if AttemptInstantDDL is set. if this.migrationContext.AttemptInstantDDL { - this.migrationContext.Log.Infof("Attempting to execute ALTER TABLE as INSTANT DDL") + this.migrationContext.Log.Infof("Attempting to execute alter with ALGORITHM=INSTANT") if err := this.attemptInstantDDL(); err == nil { - this.migrationContext.Log.Infof("Success! Table %s.%s migrated instantly", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName)) + this.migrationContext.Log.Infof("Success! table %s.%s migrated instantly", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName)) return nil } else { - this.migrationContext.Log.Infof("INSTANT DDL failed, will proceed with original algorithm: %s", err) + this.migrationContext.Log.Infof("ALGORITHM=INSTANT failed, proceeding with original algorithm: %s", err) } } diff --git a/localtests/attempt-instant-ddl/create.sql b/localtests/attempt-instant-ddl/create.sql new file mode 100644 index 0000000..9371238 --- /dev/null +++ b/localtests/attempt-instant-ddl/create.sql @@ -0,0 +1,13 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + color varchar(32), + primary key(id) +) auto_increment=1; + +drop event if exists gh_ost_test; + +insert into gh_ost_test values (null, 11, 'red'); +insert into gh_ost_test values (null, 13, 'green'); +insert into gh_ost_test values (null, 17, 'blue'); diff --git a/localtests/attempt-instant-ddl/extra_args b/localtests/attempt-instant-ddl/extra_args new file mode 100644 index 0000000..70c8a52 --- /dev/null +++ b/localtests/attempt-instant-ddl/extra_args @@ -0,0 +1 @@ +--attempt-instant-ddl From b06c1cd498b31f601abe857b0b3bd5a4737aa586 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 14 Nov 2022 12:30:38 -0700 Subject: [PATCH 5/9] Improve docs --- doc/command-line-flags.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md index b496b04..509fa67 100644 --- a/doc/command-line-flags.md +++ b/doc/command-line-flags.md @@ -47,7 +47,7 @@ You may want to use this on Amazon RDS. ### attempt-instant-ddl -MySQL 8.0 support "instant DDL" for some operations. If an alter statement can be completed with instant DDL, only a metadata change is required internally, so MySQL will return _instantly_ (only requiring a metadata lock to complete). Instant operations include: +MySQL 8.0 supports "instant DDL" for some operations. If an alter statement can be completed with instant DDL, only a metadata change is required internally. Instant operations include: - Adding a column - Dropping a column @@ -57,9 +57,9 @@ MySQL 8.0 support "instant DDL" for some operations. If an alter statement can b It is not reliable to parse the `ALTER` statement to determine if it is instant or not. This is because the table might be in an older row format, or have some other incompatibility that is difficult to identify. -The risks of attempting to instant DDL are relatively minor: Gh-ost may need to acquire a metadata lock at the start of the operation. This is not a problem for most scenarios, but it could be a problem for users that start the DDL during a period with long running transactions. +The risks of attempting instant DDL are relatively minor: `gh-ost` may need to acquire a metadata lock at the start of the operation. This is not a problem for most scenarios, but it could be a problem for users that start the DDL during a period with long running transactions. -gh-ost will automatically fallback to the normal DDL process if the attempt to use instant DDL is unsuccessful. +`gh-ost` will automatically fallback to the normal DDL process if the attempt to use instant DDL is unsuccessful. ### conf From 3f3a10a2133ae7318536f260bcdbd42ad700e2b0 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 14 Nov 2022 14:47:00 -0700 Subject: [PATCH 6/9] Address PR feedback --- go/logic/applier_test.go | 1 - go/logic/migrator.go | 23 +++++++++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/go/logic/applier_test.go b/go/logic/applier_test.go index 44d1fc7..a356351 100644 --- a/go/logic/applier_test.go +++ b/go/logic/applier_test.go @@ -182,5 +182,4 @@ func TestApplierInstantDDL(t *testing.T) { stmt := applier.generateInstantDDLQuery() test.S(t).ExpectEquals(stmt, "ALTER /* gh-ost */ TABLE `test`.`mytable` ADD INDEX (foo), ALGORITHM=INSTANT") }) - } diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 0dcf53f..31e21cc 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -351,6 +351,15 @@ func (this *Migrator) Migrate() (err error) { if err := this.initiateInspector(); err != nil { return err } + if err := this.initiateStreaming(); err != nil { + return err + } + if err := this.initiateApplier(); err != nil { + return err + } + if err := this.createFlagFiles(); err != nil { + return err + } // In MySQL 8.0 (and possibly earlier) some DDL statements can be applied instantly. // Attempt to do this if AttemptInstantDDL is set. if this.migrationContext.AttemptInstantDDL { @@ -363,16 +372,6 @@ func (this *Migrator) Migrate() (err error) { } } - if err := this.initiateStreaming(); err != nil { - return err - } - if err := this.initiateApplier(); err != nil { - return err - } - if err := this.createFlagFiles(); err != nil { - return err - } - initialLag, _ := this.inspector.getReplicationLag() this.migrationContext.Log.Infof("Waiting for ghost table to be migrated. Current lag is %+v", initialLag) <-this.ghostTableMigrated @@ -750,10 +749,6 @@ func (this *Migrator) initiateServer() (err error) { // using a ALGORITHM=INSTANT assertion. If this fails, it will return an error, // in which case the original algorithm should be used. func (this *Migrator) attemptInstantDDL() (err error) { - this.applier = NewApplier(this.migrationContext) - if err := this.applier.InitDBConnections(); err != nil { - return err - } return this.applier.AttemptInstantDDL() } From 74fb8e80b2a0f848a265b34f04ca8dbfb265743f Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 15 Nov 2022 12:05:27 -0700 Subject: [PATCH 7/9] Update go/logic/migrator.go Co-authored-by: dm-2 <45519614+dm-2@users.noreply.github.com> --- go/logic/migrator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 31e21cc..0f4ebba 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -368,7 +368,7 @@ func (this *Migrator) Migrate() (err error) { this.migrationContext.Log.Infof("Success! table %s.%s migrated instantly", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName)) return nil } else { - this.migrationContext.Log.Infof("ALGORITHM=INSTANT failed, proceeding with original algorithm: %s", err) + this.migrationContext.Log.Infof("ALGORITHM=INSTANT not supported for this operation, proceeding with original algorithm: %s", err) } } From 5283b46ec23c494b4b4256b2e5a43d0b9e4f8974 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 15 Nov 2022 12:06:11 -0700 Subject: [PATCH 8/9] Make it clear in docs it is disabled by default but safe. --- doc/command-line-flags.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md index 509fa67..56bc642 100644 --- a/doc/command-line-flags.md +++ b/doc/command-line-flags.md @@ -57,7 +57,7 @@ MySQL 8.0 supports "instant DDL" for some operations. If an alter statement can It is not reliable to parse the `ALTER` statement to determine if it is instant or not. This is because the table might be in an older row format, or have some other incompatibility that is difficult to identify. -The risks of attempting instant DDL are relatively minor: `gh-ost` may need to acquire a metadata lock at the start of the operation. This is not a problem for most scenarios, but it could be a problem for users that start the DDL during a period with long running transactions. +`--attempt-instant-ddl` is disabled by default, but the risks of enabling it are relatively minor: `gh-ost` may need to acquire a metadata lock at the start of the operation. This is not a problem for most scenarios, but it could be a problem for users that start the DDL during a period with long running transactions. `gh-ost` will automatically fallback to the normal DDL process if the attempt to use instant DDL is unsuccessful. From 59c1a24774482723f100f1213bb16b16497b40f6 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 15 Nov 2022 15:40:21 -0700 Subject: [PATCH 9/9] remove useless func per review --- go/logic/migrator.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 0f4ebba..a102188 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -364,7 +364,7 @@ func (this *Migrator) Migrate() (err error) { // Attempt to do this if AttemptInstantDDL is set. if this.migrationContext.AttemptInstantDDL { this.migrationContext.Log.Infof("Attempting to execute alter with ALGORITHM=INSTANT") - if err := this.attemptInstantDDL(); err == nil { + if err := this.applier.AttemptInstantDDL(); err == nil { this.migrationContext.Log.Infof("Success! table %s.%s migrated instantly", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName)) return nil } else { @@ -745,13 +745,6 @@ func (this *Migrator) initiateServer() (err error) { return nil } -// attemptInstantDDL tries to apply the DDL statement directly to the table -// using a ALGORITHM=INSTANT assertion. If this fails, it will return an error, -// in which case the original algorithm should be used. -func (this *Migrator) attemptInstantDDL() (err error) { - return this.applier.AttemptInstantDDL() -} - // initiateInspector connects, validates and inspects the "inspector" server. // The "inspector" server is typically a replica; it is where we issue some // queries such as: