ovn-nbctl: Add "sync" command to wait for previous changes to take effect.

It's slow to add --wait to every ovn-nbctl command; only the last command
needs it.  But it's sometimes inconvenient to add it to the last command
if it's in a loop, etc.  This makes it possible to separately wait for
the OVN southbound or hypervisors to catch up to the northbound.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Ryan Moats <rmoats@us.ibm.com>

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index d70fb106bc6f6ef7800670f0155721d352c24d3e..816cc70661419f8ca9585b6a25b7ffa89c21ae2a 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -125,6 +125,7 @@ struct ovsdb_idl_txn {
     const char *inc_table;
     const char *inc_column;
     struct uuid inc_row;
+    bool inc_force;
     unsigned int inc_index;
     int64_t inc_new_value;
 
@@ -2234,6 +2235,12 @@ ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
  * successfully, the client may retrieve the final (incremented) value of
  * 'column' with ovsdb_idl_txn_get_increment_new_value().
  *
+ * If at time of commit the transaction is otherwise empty, that is, it doesn't
+ * change the database, then 'force' is important.  If 'force' is false in this
+ * case, the IDL suppresses the increment and skips a round trip to the
+ * database server.  If 'force' is true, the IDL will still increment the
+ * column.
+ *
  * The client could accomplish something similar with ovsdb_idl_read(),
  * ovsdb_idl_txn_verify() and ovsdb_idl_txn_write(), or with ovsdb-idlc
  * generated wrappers for these functions.  However, ovsdb_idl_txn_increment()
@@ -2244,7 +2251,8 @@ ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
 void
 ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn,
                         const struct ovsdb_idl_row *row,
-                        const struct ovsdb_idl_column *column)
+                        const struct ovsdb_idl_column *column,
+                        bool force)
 {
     ovs_assert(!txn->inc_table);
     ovs_assert(column->type.key.type == OVSDB_TYPE_INTEGER);
@@ -2253,6 +2261,7 @@ ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn,
     txn->inc_table = row->table->class->name;
     txn->inc_column = column->name;
     txn->inc_row = row->uuid;
+    txn->inc_force = force;
 }
 
 /* Destroys 'txn' and frees all associated memory.  If ovsdb_idl_txn_commit()
@@ -2740,12 +2749,11 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
     }
 
     /* Add increment. */
-    if (txn->inc_table && any_updates) {
-        struct json *op;
-
+    if (txn->inc_table && (any_updates || txn->inc_force)) {
+        any_updates = true;
         txn->inc_index = operations->u.array.n - 1;
 
-        op = json_object_create();
+        struct json *op = json_object_create();
         json_object_put_string(op, "op", "mutate");
         json_object_put_string(op, "table", txn->inc_table);
         json_object_put(op, "where",

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index e25bfeffac18f01b93a55d9ba00d38510c31a3a1..45befb09319f0db9c6de216386969e52fa952014 100644 (file)
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -251,7 +251,8 @@ void ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *, const char *, ...)
 void ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_increment(struct ovsdb_idl_txn *,
                              const struct ovsdb_idl_row *,
-                             const struct ovsdb_idl_column *);
+                             const struct ovsdb_idl_column *,
+                             bool force);
 void ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *);
 enum ovsdb_idl_txn_status ovsdb_idl_txn_commit(struct ovsdb_idl_txn *);

diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 74e79a31f33be0f1ad1d2026ac39d5f65df0a012..14aa4b4300eba36cbf16ed8656bfd5e98b67fa56 100644 (file)
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -475,6 +475,22 @@
 
     <xi:include href="lib/db-ctl-base.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/>
 
+    <h1>Synchronization Commands</h1>
+
+    <dl>
+      <dt>sync</dt>
+      <dd>
+        Ordinarily, <code>--wait=sb</code> or <code>--wait=hv</code> only waits
+        for changes by the current <code>ovn-nbctl</code> invocation to take
+        effect.  This means that, if none of the commands supplied to
+        <code>ovn-nbctl</code> change the database, then the command does not
+        wait at all.  With the <code>sync</code> command, however,
+        <code>ovn-nbctl</code> waits even for earlier changes to the database
+        to propagate down to the southbound database or all of the OVN chassis,
+        according to the argument to <code>--wait</code>.
+      </dd>
+    </dl>
+
     <h1>Options</h1>
 
     <dl>
@@ -508,6 +524,15 @@
           become up-to-date with the northbound database updates.  (This can
           become an indefinite wait if any chassis is malfunctioning.)
         </p>
+
+        <p>
+          Ordinarily, <code>--wait=sb</code> or <code>--wait=hv</code> only
+          waits for changes by the current <code>ovn-nbctl</code> invocation to
+          take effect.  This means that, if none of the commands supplied to
+          <code>ovn-nbctl</code> change the database, then the command does not
+          wait at all.  Use the <code>sync</code> command to override this
+          behavior.
+        </p>
       </dd>
 
     <dt><code>--db</code> <var>database</var></dt>

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 03b6c2b255834602e5aec10fd0742552b68e19ea..9d5b1df3f15049029e143e63fd732195d83ef5b8 100644 (file)
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -57,6 +57,10 @@ enum nbctl_wait_type {
 };
 static enum nbctl_wait_type wait_type = NBCTL_WAIT_NONE;
 
+/* Should we wait (if specified by 'wait_type') even if the commands don't
+ * change the database at all? */
+static bool force_wait = false;
+
 /* --timeout: Time to wait for a connection to 'db'. */
 static int timeout;
 
@@ -408,6 +412,9 @@ DHCP Options commands:\n\
 \n\
 %s\
 \n\
+Synchronization command (use with --wait=sb|hv):\n\
+  sync                     wait even for earlier changes to take effect\n\
+\n\
 Options:\n\
   --db=DATABASE               connect to DATABASE\n\
                               (default: %s)\n\
@@ -558,6 +565,21 @@ nbctl_init(struct ctl_context *ctx OVS_UNUSED)
 {
 }
 
+static void
+nbctl_pre_sync(struct ctl_context *ctx OVS_UNUSED)
+{
+    if (wait_type != NBCTL_WAIT_NONE) {
+        force_wait = true;
+    } else {
+        VLOG_INFO("\"sync\" command has no effect without --wait");
+    }
+}
+
+static void
+nbctl_sync(struct ctl_context *ctx OVS_UNUSED)
+{
+}
+
 static void
 nbctl_show(struct ctl_context *ctx)
 {
@@ -2209,8 +2231,8 @@ do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
     }
 
     if (wait_type != NBCTL_WAIT_NONE) {
-        ovsdb_idl_txn_increment(txn, &nb->header_,
-                                &nbrec_nb_global_col_nb_cfg);
+        ovsdb_idl_txn_increment(txn, &nb->header_, &nbrec_nb_global_col_nb_cfg,
+                                force_wait);
     }
 
     symtab = ovsdb_symbol_table_create();
@@ -2394,6 +2416,7 @@ nbctl_exit(int status)
 
 static const struct ctl_command_syntax nbctl_commands[] = {
     { "init", 0, 0, "", NULL, nbctl_init, NULL, "", RW },
+    { "sync", 0, 0, "", nbctl_pre_sync, nbctl_sync, NULL, "", RO },
     { "show", 0, 1, "[SWITCH]", NULL, nbctl_show, NULL, "", RO },
 
     /* logical switch commands. */

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 4a68bca65107a1cc98dfb4bfcbe4cb7b346c45d5..f221ae8b27d5287c152d196c989eac1463b79ba0 100644 (file)
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -2120,7 +2120,8 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
                           "i=%d", atoi(arg1));
             }
 
-            ovsdb_idl_txn_increment(txn, &s->header_, &idltest_simple_col_i);
+            ovsdb_idl_txn_increment(txn, &s->header_, &idltest_simple_col_i,
+                                    false);
             increment = true;
         } else if (!strcmp(name, "abort")) {
             ovsdb_idl_txn_abort(txn);

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 722dcd9fb50f89d840e92579b4569469a8870b66..300c7fb75cee2eea6ba03179a2417ccf3916daff 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2544,7 +2544,7 @@ do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands,
 
     if (wait_for_reload) {
         ovsdb_idl_txn_increment(txn, &ovs->header_,
-                                &ovsrec_open_vswitch_col_next_cfg);
+                                &ovsrec_open_vswitch_col_next_cfg, false);
     }
 
     post_db_reload_check_init();