Support adding and satisfying constraint in same txn on replicant

Signed-off-by: Morgan Douglas <[email protected]> Tweak replicant impl Signed-off-by: Morgan Douglas <[email protected]> Tweak repl impl Signed-off-by: Morgan Douglas <[email protected]> Tweak repl impl Signed-off-by: Morgan Douglas <[email protected]>
bloomberg · Jan 7, 2025 · abd9c36 · abd9c36
1 parent 74a160e
commit abd9c36
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 13 deletions.
diff --git a/db/comdb2.c b/db/comdb2.c
@@ -2851,7 +2851,7 @@ static int db_finalize_and_sanity_checks(struct dbenv *dbenv)
             }
 
         /* verify constraint names and add reverse constraints here */
-        if (populate_reverse_constraints(NULL, db))
+        if (populate_reverse_constraints_for_single_table(NULL, db, /* track_errors */ 1))
             have_bad_schema = 1;
     }
 

diff --git a/db/comdb2.h b/db/comdb2.h
@@ -2540,7 +2540,9 @@ void dump_cache_default(void);
 int compare_all_tags(const char *table, FILE *out);
 int restore_constraint_pointers(struct dbtable *db, struct dbtable *newdb);
 int backout_constraint_pointers(struct dbtable *db, struct dbtable *newdb);
-int populate_reverse_constraints(struct ireq *iq, struct dbtable *db);
+void populate_missing_reverse_constraints_for_all_tables();
+int populate_reverse_constraints_for_single_table(struct ireq *iq, struct dbtable *db,
+                                              int track_errors);
 void init_reverse_constraints(struct dbtable *db);
 int add_reverse_constraint(struct dbtable *db, constraint_t *cnstrt);
 int delete_reverse_constraint(struct dbtable *db, size_t idx);

diff --git a/db/constraints.c b/db/constraints.c
@@ -2243,12 +2243,26 @@ int verify_constraints_exist(struct ireq *iq,
     return n_errors;
 }
 
+/*
+ * For every table constraint, adds a corresponding reverse constraint
+ * if it can be added and doesn't already exist.
+ */
+void populate_missing_reverse_constraints_for_all_tables()
+{
+    for (int i=0; i<thedb->num_dbs; ++i) {
+        (void) populate_reverse_constraints_for_single_table(NULL, thedb->dbs[i], /* track_errors */ 0);
+    }
+}
+
 /* creates a reverse constraint in the referenced table for each of the db's
  * constraint rules, if the referenced table already has the constraint a
  * duplicate is not added
- * this func also does a lot of verifications
- * returns the number of erorrs encountered */
-int populate_reverse_constraints(struct ireq *iq, struct dbtable *db)
+ * this func also does a lot of verifications.
+ * If `track_errors` is nonzero then this func
+ * returns the number of errors encountered and logs a message
+ * describing each error. */
+int populate_reverse_constraints_for_single_table(struct ireq *iq, struct dbtable *db,
+                                              int track_errors)
 {
     int ii, n_errors = 0;
 
@@ -2258,7 +2272,7 @@ int populate_reverse_constraints(struct ireq *iq, struct dbtable *db)
         struct schema *sc = NULL;
 
         sc = find_tag_schema(db, cnstrt->lclkeyname);
-        if (sc == NULL) {
+        if (sc == NULL && track_errors) {
             ++n_errors;
             logmsg(LOGMSG_ERROR,
                    "constraint error: key %s is not found in table %s\n",
@@ -2285,7 +2299,7 @@ int populate_reverse_constraints(struct ireq *iq, struct dbtable *db)
                         s = s->sc_next;
                     }
                 }
-                if (cttbl == NULL) {
+                if (cttbl == NULL && track_errors) {
                     ++n_errors;
                     logmsg(LOGMSG_ERROR, "constraint error for key %s: table %s is not found\n",
                            cnstrt->lclkeyname, cnstrt->table[jj]);
@@ -2297,11 +2311,13 @@ int populate_reverse_constraints(struct ireq *iq, struct dbtable *db)
             else
                 sckey = find_tag_schema(cttbl, cnstrt->keynm[jj]);
             if (sckey == NULL) {
-                ++n_errors;
-                logmsg(LOGMSG_ERROR, "constraint error for key %s: key %s is not found in "
+                if (track_errors) {
+                    ++n_errors;
+                    logmsg(LOGMSG_ERROR, "constraint error for key %s: key %s is not found in "
                        "table %s\n",
                        cnstrt->lclkeyname, cnstrt->keynm[jj],
                        cnstrt->table[jj]);
+                }
                 continue;
             }
 

diff --git a/schemachange/sc_add_table.c b/schemachange/sc_add_table.c
@@ -111,16 +111,48 @@ int add_table_to_environment(char *table, const char *csc2,
     newdb->timepartition_name = timepartition_name;
 
     if ((iq == NULL || iq->tranddl <= 1) && s && verify_constraints_exist(iq, newdb, NULL, NULL, s) != 0) {
+
         logmsg(LOGMSG_ERROR, "%s: failed to verify constraints\n", __func__);
         rc = -1;
         goto err;
     }
 
-    if ((iq == NULL || iq->tranddl <= 1) &&
-        populate_reverse_constraints(iq, newdb)) {
+    const int i_am_master = newdb->dbenv->master == gbl_myhostname;
+    if (i_am_master
+        && (iq == NULL || iq->tranddl <= 1)
+        && populate_reverse_constraints_for_single_table(iq, newdb, /* track_errors */ 1)) {
         logmsg(LOGMSG_ERROR, "%s: failed to populate reverse constraints\n", __func__);
         rc = -1;
         goto err;
+    } else if (!i_am_master) {
+        /*
+         * To support adding a constraint before it is satisfied in a transaction,
+         * we try to add any missing reverse constraints on every 
+         * scdone step and suppress any errors that occur in these attempts.
+         * Error suppression is acceptable since we check these conditions on master,
+         * so checking again during replication is redundant.
+         *
+         * The following is an example of a transaction that adds a constraint before
+         * it is satisfied along with an explanation of how this constraint
+         * is added during replication:
+         *
+         * begin
+         * create table t { schema { int a } keys { "a" = a }
+         *     constraints { "a" -> "q":"a" on delete cascade } }$$
+         * create table q { schema { int a } keys { "a" = a } }$$
+         * commit
+         *
+         * This transaction will have two scdone steps, one for the creation of t
+         * and another for the creation of q.
+         *
+         * During t's scdone step, we will add t's constraint on q, and we will try to 
+         * add a corresponding reverse constraint on q. This will fail, since q doesn't
+         * yet exist, and we will suppress the error.
+         *
+         * During q's scdone step, we will try to add q's missing reverse constraint.
+         * This time we will succeed since q exists.
+         */
+        populate_missing_reverse_constraints_for_all_tables();
     }
 
     if (!sc_via_ddl_only() && validate_ix_names(newdb)) {
@@ -140,7 +172,7 @@ int add_table_to_environment(char *table, const char *csc2,
         goto err;
     }
 
-    if (newdb->dbenv->master == gbl_myhostname) {
+    if (i_am_master) {
         if ((rc = sql_syntax_check(iq, newdb)))
             return SC_CSC2_ERROR;
     }
@@ -252,7 +284,8 @@ int finalize_add_table(struct ireq *iq, struct schema_change_type *s,
         sc_errf(s, "error verifying constraints\n");
         return -1;
     }
-    if (iq && iq->tranddl > 1 && populate_reverse_constraints(iq, db)) {
+    if (iq && iq->tranddl > 1
+        && populate_reverse_constraints_for_single_table(iq, db, /* track_errors */ 1)) {
         sc_errf(s, "error populating reverse constraints\n");
         return -1;
     }