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 <simon.horman@corigine.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

7 files changed, 65 insertions, 20 deletions

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index b64814076..9bad0c8dd 100644
--- a/ovsdb/ovsdb-server.c
+++ b/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;
         }
diff --git a/ovsdb/relay.c b/ovsdb/relay.c
index 94ffe01e5..377f3285f 100644
--- a/ovsdb/relay.c
+++ b/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);
diff --git a/ovsdb/relay.h b/ovsdb/relay.h
index 2d66b5e5f..f841554ca 100644
--- a/ovsdb/relay.h
+++ b/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);
 
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index f01de2a34..7cf4a851a 100644
--- a/ovsdb/transaction.c
+++ b/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 {
diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h
index 9991f34d2..0e054eef3 100644
--- a/ovsdb/transaction.h
+++ b/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(
diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c
index 0706d66cc..0edcdd89c 100644
--- a/ovsdb/trigger.c
+++ b/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
diff --git a/ovsdb/trigger.h b/ovsdb/trigger.h
index d060c72e5..87ff4d053 100644
--- a/ovsdb/trigger.h
+++ b/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);