ovn-controller: Avoid overlooking changes that occur during commit.

A commit to the database takes multiple trips through the main loop.  In
that time, the database could change, but ovn-controller can't properly
react to the changes until the commit has succeeded or failed.  Since
commit f1fd765733 (ovn-controller: Avoid blocking to commit OVSDB
transactions), Open vSwitch has failed to properly re-check the contents
of the database following a successful commit.  That meant that it was
possible for ovn-controller to fail to react to a database change until
much later, if nothing else happened for some time.

Reported-by; Alex Wang <alexw@nicira.com>
Reported-at: http://openvswitch.org/pipermail/dev/2015-July/058176.html
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 0657140..affc14b 100644 (file)
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -189,28 +189,40 @@ idl_loop_commit_and_wait(struct idl_loop *loop)
     struct ovsdb_idl_txn *txn = loop->committing_txn;
     if (txn) {
         enum ovsdb_idl_txn_status status = ovsdb_idl_txn_commit(txn);
-        switch (status) {
-        case TXN_INCOMPLETE:
-            break;
+        if (status != TXN_INCOMPLETE) {
+            switch (status) {
+            case TXN_TRY_AGAIN:
+                /* We want to re-evaluate the database when it's changed from
+                 * the contents that it had when we started the commit.  (That
+                 * might have already happened.) */
+                loop->skip_seqno = loop->precommit_seqno;
+                if (ovsdb_idl_get_seqno(loop->idl) != loop->skip_seqno) {
+                    poll_immediate_wake();
+                }
+                break;
+
+            case TXN_SUCCESS:
+                /* If the database has already changed since we started the
+                 * commit, re-evaluate it immediately to avoid missing a change
+                 * for a while. */
+                if (ovsdb_idl_get_seqno(loop->idl) != loop->precommit_seqno) {
+                    poll_immediate_wake();
+                }
+                break;
+
+            case TXN_UNCHANGED:
+            case TXN_ABORTED:
+            case TXN_NOT_LOCKED:
+            case TXN_ERROR:
+                break;
+
+            case TXN_UNCOMMITTED:
+            case TXN_INCOMPLETE:
+                OVS_NOT_REACHED();
 
-        case TXN_TRY_AGAIN:
-            loop->skip_seqno = loop->precommit_seqno;
-            if (ovsdb_idl_get_seqno(loop->idl) != loop->skip_seqno) {
-                poll_immediate_wake();
             }
-            /* Fall through. */
-        case TXN_UNCHANGED:
-        case TXN_ABORTED:
-        case TXN_SUCCESS:
-        case TXN_NOT_LOCKED:
-        case TXN_ERROR:
             ovsdb_idl_txn_destroy(txn);
             loop->committing_txn = NULL;
-            break;
-
-        case TXN_UNCOMMITTED:
-            OVS_NOT_REACHED();
-
         }
     }