Block resuming a partition merge schema change. Resuming would require sharing the destination newdb,

dorinhogea · dorinhogea · commit af0ebc88da98 · 2025-04-07T13:55:29.000-04:00
which involves more complicated code changes and it will be properly addressed in a future PR.

Signed-off-by: Dorin Hogea &lt;dhogea@bloomberg.net&gt;
diff --git a/db/osqlblockproc.c b/db/osqlblockproc.c
@@ -1471,8 +1471,9 @@ void *resume_sc_multiddl_txn_finalize(void *p)
         } else {
             logmsg(LOGMSG_ERROR, "%s: shard '%s', rc %d\n", __func__,
                    sc->tablename, sc->sc_rc);
-            sc_set_running(iq, sc, sc->tablename, 0, NULL, 0, __func__,
-                           __LINE__);
+            if (sc->set_running)
+                sc_set_running(iq, sc, sc->tablename, 0, NULL, 0, __func__,
+                               __LINE__);
             free_schema_change_type(sc);
             error = 1;
         }
diff --git a/db/osqlcomm.c b/db/osqlcomm.c
@@ -6481,14 +6481,10 @@ static int _process_partitioned_table_merge(struct ireq *iq)
          */
         sc->nothrevent = 1; /* we need do_alter_table to run first */
         sc->finalize = 0;
-        enum comdb2_partition_type tt = sc->partition.type;
-        sc->partition.type = PARTITION_NONE;
 
         strncpy(sc->tablename, first_shard->tablename, sizeof(sc->tablename));
 
         rc = start_schema_change_tran(iq, NULL);
-
-        sc->partition.type = tt;
         iq->sc->sc_next = iq->sc_pending;
         iq->sc_pending = iq->sc;
         if (rc) {
diff --git a/schemachange/sc_alter_table.c b/schemachange/sc_alter_table.c
@@ -403,7 +403,17 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
     struct scinfo scinfo;
     struct errstat err = {0};
 
-    if (s->partition.type == PARTITION_MERGE)
+    db = get_dbtable_by_name(s->tablename);
+    if (db == NULL) {
+        sc_errf(s, "Table not found:%s\n", s->tablename);
+        return SC_TABLE_DOESNOT_EXIST;
+    }
+
+
+    /* note for a partition merge, if we reuse the first shard (i.e. it is aliased),
+     * we need to alter it here instead of running do_merge_table
+     */
+    if (s->partition.type == PARTITION_MERGE && !db->sqlaliasname)
         return do_merge_table(iq, s, tran);
 
 #ifdef DEBUG_SC
@@ -413,12 +423,6 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
     gbl_use_plan = 1;
     gbl_sc_last_writer_time = 0;
 
-    db = get_dbtable_by_name(s->tablename);
-    if (db == NULL) {
-        sc_errf(s, "Table not found:%s\n", s->tablename);
-        return SC_TABLE_DOESNOT_EXIST;
-    }
-
     if (s->resume == SC_PREEMPT_RESUME) {
         newdb = db->sc_to;
         changed = s->schema_change;
diff --git a/schemachange/sc_logic.c b/schemachange/sc_logic.c
@@ -978,15 +978,58 @@ static int verify_sc_resumed_for_shard(const char *shardname,
 static int verify_sc_resumed_for_all_shards(void *obj, void *arg)
 {
     struct timepart_sc_resuming *tpt_sc = (struct timepart_sc_resuming *)obj;
+    int rc = 0;
+
+    /* corner case, shard merging: all shard schema changes need
+     * to share the same destination table newdb;
+     * at this point we will block resuming a shard merging, give
+     * the complexity of the changes involved
+     */
+    struct schema_change_type *sc =  tpt_sc->s;
+    while (sc) {
+        if (sc->partition.type == PARTITION_MERGE) {
+            break;
+        }
+        sc = sc->sc_next;
+    }
+    if (sc) {
+        /* merge operation, resume not supported */
+        logmsg(LOGMSG_ERROR, "%s partition merging detected %s, aborting\n", __func__,
+               tpt_sc->s->tablename);
+        sc = tpt_sc->s;
+        while (sc) {
+            mark_schemachange_over(sc->tablename);
+            struct dbtable *tbl = get_dbtable_by_name(sc->tablename);
+            if (tbl) {
+                sc_del_unused_files(tbl);
+            }
+            sc->sc_rc = SC_ABORTED;
+            sc = sc->sc_next;
+        }
+        return 0;
+    }
+
+    /* we need to start all the shards already in progress */
+    sc = tpt_sc->s;
+    while (sc) {
+        rc = start_schema_change(sc);
+        if (rc != SC_OK && rc != SC_ASYNC) {
+            logmsg(LOGMSG_ERROR,
+                   "%s: failed to resume schema change for table '%s' rc %d\n",
+                   __func__, sc->tablename, rc);
+            sc->sc_rc = SC_ABORTED;
+            return -1;
+        }
+        sc = sc->sc_next;
+    }
 
-    /* all shards resumed, including the next shard if any */
+    /* are all shards resumed, including the next shard if any */
     if (tpt_sc->nshards > timepart_get_num_shards(tpt_sc->viewname))
         return 0;
 
+    /* start new sc for shards that were not resumed */
     timepart_sc_arg_t sc_arg = {0};
     sc_arg.s = tpt_sc->s;
-    sc_arg.part_name = tpt_sc->viewname;
-    /* start new sc for shards that were not resumed */
     sc_arg.check_extra_shard = 1;
     sc_arg.lockless = 1;
     timepart_foreach_shard(verify_sc_resumed_for_shard, &sc_arg);
@@ -1254,19 +1297,7 @@ int resume_schema_change(void)
 
             MEMORY_SYNC;
 
-            /* start the schema change back up */
-            rc = start_schema_change(s);
-            if (rc != SC_OK) {
-                logmsg(
-                    LOGMSG_ERROR,
-                    "%s: failed to resume schema change for table '%s' rc %d\n",
-                    __func__, s->tablename, rc);
-                /* start_schema_change will free if this fails */
-                /*
-                free_schema_change_type(s);
-                */
-                continue;
-            } else if (s->timepartition_name) {
+            if (s->timepartition_name) {
                 struct timepart_sc_resuming *tpt_sc = NULL;
                 tpt_sc = hash_find(tpt_sc_hash, &s->timepartition_name);
                 if (tpt_sc == NULL) {
@@ -1287,9 +1318,23 @@ int resume_schema_change(void)
                     tpt_sc->s = s;
                     tpt_sc->nshards++;
                 }
-            } else if (s->finalize == 0 && s->kind != SC_ALTERTABLE_PENDING) {
-                s->sc_next = sc_resuming;
-                sc_resuming = s;
+            } else {
+                /* start the schema change back up */
+                rc = start_schema_change(s);
+                if (rc != SC_OK) {
+                    logmsg(
+                            LOGMSG_ERROR,
+                            "%s: failed to resume schema change for table '%s' rc %d\n",
+                            __func__, s->tablename, rc);
+                    /* start_schema_change will free if this fails */
+                    /*
+                       free_schema_change_type(s);
+                       */
+                    continue;
+                } else if (s->finalize == 0 && s->kind != SC_ALTERTABLE_PENDING) {
+                    s->sc_next = sc_resuming;
+                    sc_resuming = s;
+                }
             }
         }
     }
diff --git a/tests/TODO b/tests/TODO
@@ -83,6 +83,5 @@ rowlocks_blkseq.test
 sc_async_constraints.test
 sigstopcluster.test
 weighted_standing_queue.test -- failing in rhel8 + podman
-sc_resume_partition.test -- Pending PR #5073
 
 # vim: set sw=4 ts=4 et:
