{177357479}: Fixing an sc-resume corner case

The cluster may hang after the new master fails to resume a schema change. This patch fixes it.

Signed-off-by: Rivers Zhang <[email protected]>

schemachange/sc_alter_table.c

@@ -378,6 +378,18 @@ static int optionsChanged(struct schema_change_type *sc, struct scinfo *scinfo){
     return 0;
 }
 
+static void decrement_sc_yet_to_resume_counter()
+{
+    uint32_t oldval, newval, swapped;
+    oldval = ATOMIC_LOAD32(gbl_sc_resume_start);
+
+    /* keep performing compare-and-swap until it succeeds. also ensure that we do not go negative */
+    for (swapped = 0; oldval > 0 && !swapped; oldval = ATOMIC_LOAD32(gbl_sc_resume_start)) {
+        newval = oldval - 1;
+        swapped = CAS32(gbl_sc_resume_start, oldval, newval);
+    }
+}
+
 int do_alter_table(struct ireq *iq, struct schema_change_type *s,
                    tran_type *tran)
 {
@@ -527,6 +539,7 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
         change_schemas_recover(s->tablename);
         if (local_lock)
             unlock_schema_lk();
+        decrement_sc_yet_to_resume_counter();
         return -1;
     }
 
@@ -579,6 +592,7 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
         sc_errf(s, "failed initilizing sc_genids\n");
         delete_temp_table(iq, newdb);
         change_schemas_recover(s->tablename);
+        decrement_sc_yet_to_resume_counter();
         return -1;
     }
 
@@ -600,8 +614,7 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
                    __func__, __LINE__);
             sleep(5);
         }
-        if (gbl_sc_resume_start > 0)
-            ATOMIC_ADD32(gbl_sc_resume_start, -1);
+        decrement_sc_yet_to_resume_counter();
     }
     MEMORY_SYNC;
 

schemachange/sc_global.c

@@ -63,7 +63,8 @@ int gbl_sc_last_writer_time = 0;
 pthread_mutex_t gbl_sc_lock = PTHREAD_MUTEX_INITIALIZER;
 int gbl_sc_report_freq = 15; /* seconds between reports */
 int gbl_sc_abort = 0;
-uint32_t gbl_sc_resume_start = 0;
+/* number of schema changes that have yet to resume */
+volatile uint32_t gbl_sc_resume_start = 0;
 /* see sc_del_unused_files() and sc_del_unused_files_check_progress() */
 int sc_del_unused_files_start_ms = 0;
 int gbl_sc_del_unused_files_threshold_ms = 30000;

schemachange/sc_global.h

@@ -46,7 +46,7 @@ extern int gbl_sc_last_writer_time;
 extern pthread_mutex_t gbl_sc_lock;
 extern int gbl_sc_report_freq;      /* seconds between reports */
 extern int gbl_sc_abort;
-extern uint32_t gbl_sc_resume_start;
+extern volatile uint32_t gbl_sc_resume_start;
 /* see sc_del_unused_files() and sc_del_unused_files_check_progress() */
 extern int sc_del_unused_files_start_ms;
 extern int gbl_sc_del_unused_files_threshold_ms;

tests/sc_resume2.test/Makefile

@@ -0,0 +1,5 @@
+ifeq ($(TESTSROOTDIR),)
+  include ../testcase.mk
+else
+  include $(TESTSROOTDIR)/testcase.mk
+endif

tests/sc_resume2.test/lrl.options

@@ -0,0 +1,2 @@
+setattr MEMPTRICKLEPERCENT 50
+logmsg level info

tests/sc_resume2.test/runit

@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+# simple test to verify that new master does not block indefinitely when
+# resuming a schema change that made zero progress from the previous master
+
+source ${TESTSROOTDIR}/tools/runit_common.sh
+source ${TESTSROOTDIR}/tools/cluster_utils.sh
+
+bash -n "$0" | exit 1
+[ -z "${CLUSTER}" ] && { echo "Test requires a cluster"; exit 0; }
+
+dbnm=$1
+master=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default 'SELECT host FROM comdb2_cluster WHERE is_master="Y"'`
+
+cdb2sql ${CDB2_OPTIONS} $dbnm default "CREATE TABLE t1 (a INTEGER)"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "INSERT INTO t1 VALUES (1)"
+
+for node in $CLUSTER ; do
+    cdb2sql $dbnm --host $node "EXEC PROCEDURE sys.cmd.send('on rep_delay')"
+done
+
+cdb2sql $dbnm --host $master "EXEC PROCEDURE sys.cmd.send('convert_record_sleep 1')"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "REBUILD t1" &
+waitpid=$!
+sleep 2 # give the master node a bit time to get to the convert thread
+for node in $CLUSTER ; do
+    kill_restart_node $node &
+done
+
+wait $waitpid
+sleep 5
+cdb2sql ${CDB2_OPTIONS} $dbnm default "INSERT INTO t1 VALUES (1)"