Merge branch 'master' into grodowski/coding-chimp/on-retry-hook

Author: grodowski

doc/command-line-flags.md

@@ -24,6 +24,11 @@ By default, `gh-ost` would like you to connect to a replica, from where it figur
 
 If, for some reason, you do not wish `gh-ost` to connect to a replica, you may connect it directly to the master and approve this via `--allow-on-master`.
 
+### allow-setup-metadata-lock-instruments
+
+`--allow-setup-metadata-lock-instruments` allows gh-ost to enable the [`metadata_locks`](https://dev.mysql.com/doc/refman/8.0/en/performance-schema-metadata-locks-table.html) table in `performance_schema`, if it is not already enabled. This is used for a safety check before cut-over.
+See also: [`skip-metadata-lock-check`](#skip-metadata-lock-check)
+
 ### approve-renamed-columns
 
 When your migration issues a column rename (`change column old_name new_name ...`) `gh-ost` analyzes the statement to try and associate the old column name with new column name. Otherwise, the new structure may also look like some column was dropped and another was added.
@@ -247,6 +252,13 @@ Defaults to an auto-determined and advertised upon startup file. Defines Unix so
 
 By default `gh-ost` verifies no foreign keys exist on the migrated table. On servers with large number of tables this check can take a long time. If you're absolutely certain no foreign keys exist (table does not reference other table nor is referenced by other tables) and wish to save the check time, provide with `--skip-foreign-key-checks`.
 
+### skip-metadata-lock-check
+
+By default `gh-ost` performs a check before the cut-over to ensure the rename session holds the exclusive metadata lock on the table. In case `performance_schema.metadata_locks` cannot be enabled on your setup, this check can be skipped with `--skip-metadata-lock-check`. 
+:warning: Disabling this check involves the small chance of data loss in case a session accesses the ghost table during cut-over. See https://github.com/github/gh-ost/pull/1536 for details.
+
+See also: [`allow-setup-metadata-lock-instruments`](#allow-setup-metadata-lock-instruments)
+
 ### skip-strict-mode
 
 By default `gh-ost` enforces STRICT_ALL_TABLES sql_mode as a safety measure. In some cases this changes the behaviour of other modes (namely ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, and NO_ZERO_IN_DATE) which may lead to errors during migration. Use `--skip-strict-mode` to explicitly tell `gh-ost` not to enforce this. **Danger** This may have some unexpected disastrous side effects.

go/base/context.go

@@ -256,6 +256,7 @@ type MigrationContext struct {
 
 	BinlogSyncerMaxReconnectAttempts  int
 	AllowSetupMetadataLockInstruments bool
+	SkipMetadataLockCheck             bool
 	IsOpenMetadataLockInstruments     bool
 
 	Log Logger

go/cmd/gh-ost/main.go

@@ -138,7 +138,8 @@ func main() {
 	flag.Int64Var(&migrationContext.HooksStatusIntervalSec, "hooks-status-interval", 60, "how many seconds to wait between calling onStatus hook")
 
 	flag.UintVar(&migrationContext.ReplicaServerId, "replica-server-id", 99999, "server id used by gh-ost process. Default: 99999")
-	flag.BoolVar(&migrationContext.AllowSetupMetadataLockInstruments, "allow-setup-metadata-lock-instruments", false, "validate rename session hold the MDL of original table before unlock tables in cut-over phase")
+	flag.BoolVar(&migrationContext.AllowSetupMetadataLockInstruments, "allow-setup-metadata-lock-instruments", false, "Validate rename session hold the MDL of original table before unlock tables in cut-over phase")
+	flag.BoolVar(&migrationContext.SkipMetadataLockCheck, "skip-metadata-lock-check", false, "Skip metadata lock check at cut-over time. The checks require performance_schema.metadata_lock to be enabled")
 	flag.IntVar(&migrationContext.BinlogSyncerMaxReconnectAttempts, "binlogsyncer-max-reconnect-attempts", 0, "when master node fails, the maximum number of binlog synchronization attempts to reconnect. 0 is unlimited")
 
 	flag.BoolVar(&migrationContext.IncludeTriggers, "include-triggers", false, "When true, the triggers (if exist) will be created on the new table")

go/logic/applier.go

@@ -483,10 +483,16 @@ func (this *Applier) dropTable(tableName string) error {
 	return nil
 }
 
+// StateMetadataLockInstrument checks if metadata_locks is enabled in performance_schema.
+// If not it attempts to enable metadata_locks if this is allowed.
 func (this *Applier) StateMetadataLockInstrument() error {
 	query := `select /*+ MAX_EXECUTION_TIME(300) */ ENABLED, TIMED from performance_schema.setup_instruments WHERE NAME = 'wait/lock/metadata/sql/mdl'`
 	var enabled, timed string
 	if err := this.db.QueryRow(query).Scan(&enabled, &timed); err != nil {
+		if errors.Is(err, gosql.ErrNoRows) {
+			// performance_schema may be disabled.
+			return nil
+		}
 		return this.migrationContext.Log.Errorf("query performance_schema.setup_instruments with name wait/lock/metadata/sql/mdl error: %s", err)
 	}
 	if strings.EqualFold(enabled, "YES") && strings.EqualFold(timed, "YES") {
@@ -1340,7 +1346,7 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke
 
 	this.migrationContext.Log.Infof("Session renameLockSessionId is %+v", *renameLockSessionId)
 	// Checking the lock is held by rename session
-	if *renameLockSessionId > 0 && this.migrationContext.IsOpenMetadataLockInstruments {
+	if *renameLockSessionId > 0 && this.migrationContext.IsOpenMetadataLockInstruments && !this.migrationContext.SkipMetadataLockCheck {
 		sleepDuration := time.Duration(10*this.migrationContext.CutOverLockTimeoutSeconds) * time.Millisecond
 		for i := 1; i <= 100; i++ {
 			err := this.ExpectMetadataLock(*renameLockSessionId)

go/logic/migrator.go

@@ -1376,10 +1376,18 @@ func (this *Migrator) initiateApplier() error {
 		}
 		this.applier.WriteChangelogState(string(GhostTableMigrated))
 	}
+
+	// ensure performance_schema.metadata_locks is available.
 	if err := this.applier.StateMetadataLockInstrument(); err != nil {
-		this.migrationContext.Log.Errorf("Unable to enable metadata lock instrument, see further error details. Bailing out")
-		return err
+		this.migrationContext.Log.Warning("Unable to enable metadata lock instrument, see further error details.")
 	}
+	if !this.migrationContext.IsOpenMetadataLockInstruments {
+		if !this.migrationContext.SkipMetadataLockCheck {
+			return this.migrationContext.Log.Errorf("Bailing out because metadata lock instrument not enabled. Use --skip-metadata-lock-check if you wish to proceed without. See https://github.com/github/gh-ost/pull/1536 for details.")
+		}
+		this.migrationContext.Log.Warning("Proceeding without metadata lock check. There is a small chance of data loss if another session accesses the ghost table during cut-over. See https://github.com/github/gh-ost/pull/1536 for details.")
+	}
+
 	go this.applier.InitiateHeartbeat()
 	return nil
 }

localtests/test.sh

@@ -292,6 +292,7 @@ test_single() {
     --alter='engine=${storage_engine}' \
     --exact-rowcount \
     --assume-rbr \
+    --skip-metadata-lock-check \
     --initially-drop-old-table \
     --initially-drop-ghost-table \
     --throttle-query='select timestampdiff(second, min(last_update), now()) < 5 from _${table_name}_ghc' \