address feedback

Author: chriskirkland

go/logic/migrator.go

@@ -1386,7 +1386,7 @@ func (mgtr *Migrator) moveTablesCutOver() (err error) {
 		}
 	}
 
-	mgtr.migrationContext.NewFailPoint("panic-before-drain-completion", base.WithFailPointWait(2*time.Second))
+	mgtr.migrationContext.NewFailPoint("move-tables-panic-before-drain-completion", base.WithFailPointWait(2*time.Second))
 
 	// ------ T3: draining applier to drain GTID -----------
 	if err := mgtr.drainMoveTablesCutOver(drainGTID); err != nil {
@@ -1406,7 +1406,7 @@ func (mgtr *Migrator) moveTablesCutOver() (err error) {
 	atomic.StoreInt64(&mgtr.migrationContext.CutOverCompleteFlag, 1)
 	mgtr.migrationContext.Log.Debugf("T4: CutOverCompleteFlag set")
 
-	mgtr.migrationContext.NewFailPoint("panic-before-on-success-hook", base.WithFailPointWait(2*time.Second))
+	mgtr.migrationContext.NewFailPoint("move-tables-panic-before-on-success-hook", base.WithFailPointWait(2*time.Second))
 
 	// ----- T5: on-success hook -----
 	// Hook unlocks user_rw@target via db-user-management and flips the
@@ -2515,7 +2515,7 @@ func (mgtr *Migrator) iterateChunks() error {
 			return terminateRowIteration(err)
 		}
 
-		mgtr.migrationContext.NewFailPoint("panic-after-row-copy", base.WithFailPointWait(2*time.Second))
+		mgtr.migrationContext.NewFailPoint("move-tables-panic-after-row-copy", base.WithFailPointWait(2*time.Second))
 	}
 }
 

localtests/move-tables/resume-panic-before-drain-complete/test.sh

@@ -21,7 +21,7 @@ build_binary
 echo  "⚙️ Starting migration with failpoint (run #1)..."
 
 # Build the gh-ost command using the framework function
-GO_FAILPOINTS="github.com/github/gh-ost/go/base/panic-before-drain-completion=return(true)" build_ghost_command
+GO_FAILPOINTS="github.com/github/gh-ost/go/base/move-tables-panic-before-drain-completion=return(true)" build_ghost_command
 
 # queue up removal of the postpone cutover flag, otherwise gh-ost hangs on the cutover
 (
@@ -76,6 +76,19 @@ if [ $? -gt 0 ]; then
     return 1
 fi
 
+# validate last checkpoint (cutover started and drain GTID are set)
+cutover_started=$(mysql-exec target primary $database -Ne "SELECT gh_ost_move_tables_cutover_started FROM _${table_name}_ghk ORDER BY gh_ost_chk_id DESC LIMIT 1;")
+if [ "$cutover_started" != 1 ]; then
+    echo "ERROR: Expected cutover started to be set in last checkpoint."
+    return 1
+fi
+
+drain_gtid=$(mysql-exec target primary $database -Ne "SELECT gh_ost_move_tables_drain_gtid FROM _${table_name}_ghk ORDER BY gh_ost_chk_id DESC LIMIT 1;")
+if [ "$drain_gtid" == "" ]; then
+    echo "ERROR: Expected drain GTID to be set in last checkpoint."
+    return 1
+fi
+
 echo  "✅ Validated checkpointed state on unexpected exit..."
 
 echo -e "\n\n\n\n\n"

localtests/move-tables/resume-panic-before-on-success-hook/hooks/gh-ost-on-success

@@ -0,0 +1,4 @@
+#!/bin/bash
+
+# touch file to mark completion of on-success hook
+touch /tmp/gh-ost-hooks/on-success

localtests/move-tables/resume-panic-before-on-success-hook/test.sh

@@ -13,14 +13,23 @@ table_name=gh_ost_test
 rm $ghost_binary
 build_binary
 
+# ensure hook files are executable
+chmod +x $tests_path/$test_name/hooks/*
+
+# clean up any existing test hook files
+rm -rf /tmp/gh-ost-hooks/
+mkdir -p /tmp/gh-ost-hooks/
+
 ######################################################################################################
 ### Run #1: Should panic after drain (T4) and before on-success (T5)
 ######################################################################################################
 
+
 echo  "⚙️ Starting migration with failpoint (run #1)..."
 
 # Build the gh-ost command using the framework function
-GO_FAILPOINTS="github.com/github/gh-ost/go/base/panic-before-on-success-hook=return(true)" build_ghost_command
+GO_FAILPOINTS="github.com/github/gh-ost/go/base/move-tables-panic-before-on-success-hook=return(true)" build_ghost_command
+cmd="$cmd --hooks-path=$tests_path/$test_name/hooks"
 
 # queue up removal of the postpone cutover flag, otherwise gh-ost hangs on the cutover
 (
@@ -84,8 +93,8 @@ if [ $? -gt 0 ]; then
 fi
 
 # contents of table on source and target are the same
-source_contents_file=/tmp/gh-ost-test.resume-panic-before-on-success-hook-source_contents.txt
-target_contents_file=/tmp/gh-ost-test.resume-panic-before-on-success-hook-target_contents.txt
+source_contents_file=/tmp/gh-ost-test.resume-move-tables-panic-before-on-success-hook-source_contents.txt
+target_contents_file=/tmp/gh-ost-test.resume-move-tables-panic-before-on-success-hook-target_contents.txt
 mysql-exec source primary $database -sNe "SELECT * FROM _${table_name}_del;" > $source_contents_file
 mysql-exec target primary $database -sNe "SELECT * FROM ${table_name};" > $target_contents_file
 
@@ -97,6 +106,12 @@ if ! diff $source_contents_file $target_contents_file; then
     return 1
 fi
 
+# validate on-success hook was not called
+if [ -f /tmp/gh-ost-hooks/on-success ]; then
+    echo "ERROR: on-success hook was called when it should not have been."
+    return 1
+fi
+
 echo  "✅ Validated checkpointed state on unexpected exit..."
 
 echo -e "\n\n\n\n\n"
@@ -109,7 +124,7 @@ echo  "⚙️ Resuming migration (run #2)..."
 
 # resume migration
 build_ghost_command
-cmd="$cmd --resume"
+cmd="$cmd --resume --hooks-path=$tests_path/$test_name/hooks"
 
 # queue up removal of the postpone cutover flag, otherwise gh-ost hangs on the cutover
 (
@@ -126,4 +141,9 @@ if [ $ghost_result -ne 0 ]; then
     return 1
 fi
 
+# validate on-success hook was was called
+if [ ! -f /tmp/gh-ost-hooks/on-success ]; then
+    echo "ERROR: on-success hook was not called when it should have been."
+fi
+
 echo -e "\n\n\n\n\n"

localtests/move-tables/resume-panic-on-row-copy/test.sh

@@ -18,7 +18,7 @@ build_binary
 echo  "⚙️ Starting migration with failpoint (run #1)..."
 
 # Build the gh-ost command using the framework function
-GO_FAILPOINTS="github.com/github/gh-ost/go/base/panic-after-row-copy=return(true)" build_ghost_command
+GO_FAILPOINTS="github.com/github/gh-ost/go/base/move-tables-panic-after-row-copy=return(true)" build_ghost_command
 
 # Run the gh-ost command, expecting panic on the failpoint the first time
 echo_dot
@@ -39,10 +39,10 @@ echo -e "\n\n\n\n\n"
 
 echo  "⚙️ Validating checkpointed state on unexpected exit..."
 
-# checkpoint file exists on target and is non-empty
+# checkpoint table exists on target and is non-empty
 mysql-exec target primary $database -sNe "SELECT 1 FROM _${table_name}_ghk LIMIT 1;"
 if [ $? -gt 0 ]; then
-    echo "ERROR: Checkpoint file is empty or does not exist."
+    echo "ERROR: Checkpoint table is empty or does not exist."
     return 1
 fi
 
@@ -60,6 +60,13 @@ if [ $? -gt 0 ]; then
     return 1
 fi
 
+# validate we processed a single row-copy chunk (10 rows) and there are 20 total to process
+rows_copied=$(mysql-exec target primary $database -Ne "SELECT gh_ost_rows_copied FROM _${table_name}_ghk ORDER BY gh_ost_chk_id DESC LIMIT 1;")
+if [ $rows_copied -ne 10 ]; then
+    echo "ERROR: Expected last checkpoint to show 10 rows copied."
+    return 1
+fi
+
 echo  "✅ Validating checkpointed state on unexpected exit..."
 
 echo -e "\n\n\n\n\n"
@@ -90,3 +97,21 @@ if [ $ghost_result -ne 0 ]; then
 fi
 
 echo -e "\n\n\n\n\n"
+
+######################################################################################################
+### post-migration validation
+######################################################################################################
+
+echo  "⚙️ Validating checkpointed state after resumed migration..."
+
+# validate we processed a single row-copy chunk (10 rows) and there are 20 total to process
+rows_copied=$(mysql-exec target primary $database -Ne "SELECT gh_ost_rows_copied FROM _${table_name}_ghk ORDER BY gh_ost_chk_id DESC LIMIT 1;")
+if [ $rows_copied -ne 20 ]; then
+    echo "ERROR: Expected last checkpoint to show 20 rows copied."
+    return 1
+fi
+
+echo  "✅ Validating checkpointed state on resumed migration."
+
+echo -e "\n\n\n\n\n"
+

localtests/move-tables/single-with-hooks/hooks/gh-ost-on-before-cut-over

@@ -10,5 +10,5 @@ set -e
 
 assert_common_envs
 
-# touch file to mark completion of on-row-copy-complete hook
+# touch file to mark completion of on-before-cut-over hook
 touch /tmp/gh-ost-hooks/on-before-cut-over

localtests/move-tables/single-with-hooks/hooks/gh-ost-on-success

@@ -12,5 +12,5 @@ assert_common_envs
 
 assert_env_present "GH_OST_DRAIN_GTID"
 
-# touch file to mark completion of on-row-copy-complete hook
+# touch file to mark completion of on-success hook
 touch /tmp/gh-ost-hooks/on-success