diff mbox

[ovs-dev,01/30] ovn-nbctl: Add "sync" command to wait for previous changes to take effect.

Message ID 1470642309-18995-2-git-send-email-blp@ovn.org
State Accepted
Headers show

Commit Message

Ben Pfaff Aug. 8, 2016, 7:44 a.m. UTC
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>
---
 lib/ovsdb-idl.c               | 18 +++++++++++++-----
 lib/ovsdb-idl.h               |  3 ++-
 ovn/utilities/ovn-nbctl.8.xml | 25 +++++++++++++++++++++++++
 ovn/utilities/ovn-nbctl.c     | 27 +++++++++++++++++++++++++--
 tests/test-ovsdb.c            |  5 +++--
 utilities/ovs-vsctl.c         |  2 +-
 6 files changed, 69 insertions(+), 11 deletions(-)

Comments

Ryan Moats Aug. 8, 2016, 3:08 p.m. UTC | #1
"dev" <dev-bounces@openvswitch.org> wrote on 08/08/2016 02:44:40 AM:

> From: Ben Pfaff <blp@ovn.org>
> To: dev@openvswitch.org
> Cc: Ben Pfaff <blp@ovn.org>
> Date: 08/08/2016 02:45 AM
> Subject: [ovs-dev] [PATCH 01/30] ovn-nbctl: Add "sync" command to
> wait for previous changes to take effect.
> Sent by: "dev" <dev-bounces@openvswitch.org>
>
> 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>
> ---

FYI, since there are a lot of these, I'm going to fall back to bare acks
unless I see something odd...

Acked-by: Ryan Moats <rmoats@us.ibm.com>
diff mbox

Patch

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index d70fb10..816cc70 100644
--- 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 e25bfef..45befb0 100644
--- 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 122a114..11a5a46 100644
--- 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 3d74779..1066429 100644
--- 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\
@@ -559,6 +566,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)
 {
     const struct nbrec_logical_switch *ls;
@@ -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 4a68bca..f221ae8 100644
--- 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 722dcd9..300c7fb 100644
--- 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();