ovsdb: Avoid converting database twice on an initiator.

Cluster member, that initiates the schema conversion, converts the
database twice.  First time while verifying the possibility of the
conversion, and the second time after reading conversion request
back from the storage.

Keep the converted database from the first time around and use it
after reading the request back from the storage.  This cuts in half
the conversion CPU cost.

Reviewed-by: Simon Horman <[email protected]>
Acked-by: Dumitru Ceara <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>

ovsdb/ovsdb-server.c

@@ -574,7 +574,9 @@ close_db(struct server_config *config, struct db *db, char *comment)
 }
 
 static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
-update_schema(struct ovsdb *db, const struct ovsdb_schema *schema,
+update_schema(struct ovsdb *db,
+              const struct ovsdb_schema *schema,
+              const struct uuid *txnid,
               bool conversion_with_no_data, void *aux)
 {
     struct server_config *config = aux;
@@ -591,11 +593,17 @@ update_schema(struct ovsdb *db, const struct ovsdb_schema *schema,
         struct ovsdb *new_db = NULL;
         struct ovsdb_error *error;
 
-        error = ovsdb_convert(db, schema, &new_db);
-        if (error) {
-            /* Should never happen, because conversion should have been
-             * checked before writing the schema to the storage. */
-            return error;
+        /* If conversion was triggered by the current process, we might
+         * already have converted version of a database. */
+        new_db = ovsdb_trigger_find_and_steal_converted_db(db, txnid);
+        if (!new_db) {
+            /* No luck.  Converting. */
+            error = ovsdb_convert(db, schema, &new_db);
+            if (error) {
+                /* Should never happen, because conversion should have been
+                 * checked before writing the schema to the storage. */
+                return error;
+            }
         }
         ovsdb_replace(db, new_db);
     } else {
@@ -635,7 +643,7 @@ parse_txn(struct server_config *config, struct db *db,
             return error;
         }
 
-        error = update_schema(db->db, schema, txn_json == NULL, config);
+        error = update_schema(db->db, schema, txnid, txn_json == NULL, config);
         if (error) {
             return error;
         }

ovsdb/relay.c

@@ -310,8 +310,8 @@ ovsdb_relay_parse_update(struct relay_ctx *ctx,
     if (update->monitor_reply && ctx->new_schema) {
         /* There was a schema change.  Updating a database with a new schema
          * before processing monitor reply with the new data. */
-        error = ctx->schema_change_cb(ctx->db, ctx->new_schema, false,
-                                      ctx->schema_change_aux);
+        error = ctx->schema_change_cb(ctx->db, ctx->new_schema, &UUID_ZERO,
+                                      false, ctx->schema_change_aux);
         if (error) {
             /* Should never happen, but handle this case anyway. */
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);

ovsdb/relay.h

@@ -22,10 +22,12 @@
 struct json;
 struct ovsdb;
 struct ovsdb_schema;
+struct uuid;
 
 typedef struct ovsdb_error *(*schema_change_callback)(
                                        struct ovsdb *,
                                        const struct ovsdb_schema *,
+                                       const struct uuid *,
                                        bool conversion_with_no_data,
                                        void *aux);
 

ovsdb/transaction.c

@@ -1252,14 +1252,14 @@ ovsdb_txn_precheck_prereq(const struct ovsdb *db)
 struct ovsdb_txn_progress *
 ovsdb_txn_propose_schema_change(struct ovsdb *db,
                                 const struct ovsdb_schema *schema,
-                                const struct json *data)
+                                const struct json *data,
+                                struct uuid *txnid)
 {
     struct ovsdb_txn_progress *progress = xzalloc(sizeof *progress);
     progress->storage = db->storage;
 
-    struct uuid next;
     struct ovsdb_write *write = ovsdb_storage_write_schema_change(
-        db->storage, schema, data, &db->prereq, &next);
+        db->storage, schema, data, &db->prereq, txnid);
     if (!ovsdb_write_is_complete(write)) {
         progress->write = write;
     } else {

ovsdb/transaction.h

@@ -42,7 +42,8 @@ struct ovsdb_error *ovsdb_txn_propose_commit_block(struct ovsdb_txn *,
 void ovsdb_txn_complete(struct ovsdb_txn *);
 
 struct ovsdb_txn_progress *ovsdb_txn_propose_schema_change(
-    struct ovsdb *, const struct ovsdb_schema *, const struct json *data);
+    struct ovsdb *, const struct ovsdb_schema *,
+    const struct json *data, struct uuid *txnid);
 
 bool ovsdb_txn_progress_is_complete(const struct ovsdb_txn_progress *);
 const struct ovsdb_error *ovsdb_txn_progress_get_error(

ovsdb/trigger.c

@@ -31,6 +31,7 @@
 #include "transaction-forward.h"
 #include "openvswitch/vlog.h"
 #include "util.h"
+#include "uuid.h"
 
 VLOG_DEFINE_THIS_MODULE(trigger);
 
@@ -52,6 +53,7 @@ ovsdb_trigger_init(struct ovsdb_session *session, struct ovsdb *db,
     trigger->db = db;
     ovs_list_push_back(&trigger->db->triggers, &trigger->node);
     trigger->request = request;
+    trigger->converted_db = NULL;
     trigger->reply = NULL;
     trigger->progress = NULL;
     trigger->txn_forward = NULL;
@@ -69,6 +71,7 @@ ovsdb_trigger_destroy(struct ovsdb_trigger *trigger)
     ovsdb_txn_progress_destroy(trigger->progress);
     ovsdb_txn_forward_destroy(trigger->db, trigger->txn_forward);
     ovs_list_remove(&trigger->node);
+    ovsdb_destroy(trigger->converted_db);
     jsonrpc_msg_destroy(trigger->request);
     jsonrpc_msg_destroy(trigger->reply);
     free(trigger->role);
@@ -143,6 +146,30 @@ ovsdb_trigger_prereplace_db(struct ovsdb_trigger *trigger)
     }
 }
 
+/* Find among incomplete triggers one that caused database conversion
+ * with specified transaction ID. */
+struct ovsdb *
+ovsdb_trigger_find_and_steal_converted_db(const struct ovsdb *db,
+                                          const struct uuid *txnid)
+{
+    struct ovsdb *converted_db = NULL;
+    struct ovsdb_trigger *t;
+
+    if (uuid_is_zero(txnid)) {
+        return NULL;
+    }
+
+    LIST_FOR_EACH_SAFE (t, node, &db->triggers) {
+        if (t->db == db && t->converted_db
+            && uuid_equals(&t->conversion_txnid, txnid)) {
+            converted_db = t->converted_db;
+            t->converted_db = NULL;
+            break;
+        }
+    }
+    return converted_db;
+}
+
 bool
 ovsdb_trigger_run(struct ovsdb *db, long long int now)
 {
@@ -200,7 +227,6 @@ ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
         ovs_assert(!t->progress);
 
         struct ovsdb_txn *txn = NULL;
-        struct ovsdb *newdb = NULL;
         if (!strcmp(t->request->method, "transact")) {
             if (!ovsdb_txn_precheck_prereq(t->db)) {
                 return false;
@@ -272,7 +298,8 @@ ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
                                     new_schema->name, t->db->schema->name);
             }
             if (!error) {
-                error = ovsdb_convert(t->db, new_schema, &newdb);
+                ovsdb_destroy(t->converted_db);
+                error = ovsdb_convert(t->db, new_schema, &t->converted_db);
             }
             if (error) {
                 ovsdb_schema_destroy(new_schema);
@@ -286,12 +313,12 @@ ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
             } else {
                 /* Make the new copy into a transaction log record. */
                 txn_json = ovsdb_to_txn_json(
-                                newdb, "converted by ovsdb-server", true);
+                    t->converted_db, "converted by ovsdb-server", true);
             }
 
             /* Propose the change. */
             t->progress = ovsdb_txn_propose_schema_change(
-                t->db, new_schema, txn_json);
+                t->db, new_schema, txn_json, &t->conversion_txnid);
             ovsdb_schema_destroy(new_schema);
             json_destroy(txn_json);
             t->reply = jsonrpc_create_reply(json_object_create(),
@@ -313,13 +340,13 @@ ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
             ovsdb_txn_progress_destroy(t->progress);
             t->progress = NULL;
             ovsdb_trigger_complete(t);
-            if (newdb) {
-                ovsdb_replace(t->db, newdb);
+            if (t->converted_db) {
+                ovsdb_replace(t->db, t->converted_db);
+                t->converted_db = NULL;
                 return true;
             }
             return false;
         }
-        ovsdb_destroy(newdb);
 
         /* Fall through to the general handling for the "committing" state.  We
          * abort the transaction--if and when it eventually commits, we'll read

ovsdb/trigger.h

@@ -17,6 +17,7 @@
 #define OVSDB_TRIGGER_H 1
 
 #include "openvswitch/list.h"
+#include "openvswitch/uuid.h"
 
 struct ovsdb;
 
@@ -54,6 +55,8 @@ struct ovsdb_trigger {
     struct ovs_list node;
     struct ovsdb_session *session; /* Session that owns this trigger. */
     struct ovsdb *db;           /* Database on which trigger acts. */
+    struct ovsdb *converted_db;   /* Result of the 'convert' request. */
+    struct uuid conversion_txnid; /* txnid of the conversion request. */
     struct jsonrpc_msg *request; /* Database request. */
     struct jsonrpc_msg *reply;   /* Result (null if none yet). */
     struct ovsdb_txn_progress *progress;
@@ -77,6 +80,10 @@ void ovsdb_trigger_cancel(struct ovsdb_trigger *, const char *reason);
 
 void ovsdb_trigger_prereplace_db(struct ovsdb_trigger *);
 
+struct ovsdb *ovsdb_trigger_find_and_steal_converted_db(
+        const struct ovsdb *, const struct uuid *)
+    OVS_WARN_UNUSED_RESULT;
+
 bool ovsdb_trigger_run(struct ovsdb *, long long int now);
 void ovsdb_trigger_wait(struct ovsdb *, long long int now);