When reconnecting, if there are condition changes already sent to the
server but not yet acked, reset the db's 'last-id', esentially clearing
the local cache after reconnect.

This is needed because the client cannot easily differentiate between
the following cases:
a. either the server already processed the requested monitor
   condition change but the FSM was restarted before the
   client was notified.  In this case the client should
   clear its local cache because it's out of sync with the
   monitor view on the server side.
b. OR the server hasn't processed the requested monitor
   condition change yet.

Conditions changing at the same time with a reconnection happening are
rare so the performance impact of this patch should be minimal.

Also, the tests are updated to cover the fact that we cannot control
which of the two scenarios ("a" and "b" above) are hit during the test.

Reported-by: Maxime Coquelin <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
---
Note: The python IDL is fixed in the following patch in order to make
this patch easier to backport.  test-ovsdb.py is updated in this patch
to avoid having the python version of the test perma-failing if the
last patch in the series is not applied.
---
 lib/ovsdb-cs.c      |   17 ++++++++++
 tests/ovsdb-idl.at  |    4 +-
 tests/test-ovsdb.c  |   89 +++++++++++++++++++++++++++++----------------------
 tests/test-ovsdb.py |   46 ++++++++++++++++++--------
 4 files changed, 102 insertions(+), 54 deletions(-)

diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index fcb6fe1b34be..a4457245aa2d 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -1109,6 +1109,23 @@ ovsdb_cs_db_sync_condition(struct ovsdb_cs_db *db)
                 }
                 table->req_cond = NULL;
                 db->cond_changed = true;
+
+                /* There are two cases:
+                 * a. either the server already processed the requested monitor
+                 *    condition change but the FSM was restarted before the
+                 *    client was notified.  In this case the client should
+                 *    clear its local cache because it's out of sync with the
+                 *    monitor view on the server side.
+                 *
+                 * b. OR the server hasn't processed the requested monitor
+                 *    condition change yet.
+                 *
+                 * As there's no easy way to differentiate between the two,
+                 * and given that this condition should be rare, reset the
+                 * 'last_id', essentially flushing the local cached DB
+                 * contents.
+                 */
+                db->last_id = UUID_ZERO;
             }
         }
     }
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 86a75f920379..62e2b638320c 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2336,7 +2336,7 @@ OVSDB_CHECK_CLUSTER_IDL([simple idl, monitor_cond_since, 
cluster disconnect],
     'condition simple [["i","==",2]]' \
     'condition simple [["i","==",1]]' \
     '+reconnect' \
-    '["idltest",
+    '?["idltest",
       {"op": "update",
        "table": "simple",
        "where": [["i", "==", 1]],
@@ -2347,7 +2347,7 @@ OVSDB_CHECK_CLUSTER_IDL([simple idl, monitor_cond_since, 
cluster disconnect],
 003: table simple: i=2 r=1 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] 
uuid=<1>
 004: change conditions
 005: reconnect
-006: table simple: i=2 r=1 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] 
uuid=<1>
+006: table simple
 007: {"error":null,"result":[{"count":1}]}
 008: table simple: i=1 r=2 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] 
uuid=<2>
 009: done
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index a40318c7dba9..ca4e87b8115c 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1879,7 +1879,8 @@ print_and_log(const char *format, ...)
 }
 
 static char *
-format_idl_row(const struct ovsdb_idl_row *row, int step, const char *contents)
+format_idl_row(const struct ovsdb_idl_row *row, int step, const char *contents,
+               bool terse)
 {
     const char *change_str =
         !ovsdb_idl_track_is_set(row->table)
@@ -1890,9 +1891,13 @@ format_idl_row(const struct ovsdb_idl_row *row, int 
step, const char *contents)
             ? "deleted row: "
             : "";
 
-    return xasprintf("%03d: table %s: %s%s uuid=" UUID_FMT,
-                     step, row->table->class_->name, change_str, contents,
-                     UUID_ARGS(&row->uuid));
+    if (terse) {
+        return xasprintf("%03d: table %s", step, row->table->class_->name);
+    } else {
+        return xasprintf("%03d: table %s: %s%s uuid=" UUID_FMT,
+                         step, row->table->class_->name, change_str,
+                         contents, UUID_ARGS(&row->uuid));
+    }
 }
 
 static void
@@ -2015,7 +2020,7 @@ print_idl_row_updated_singleton(const struct 
idltest_singleton *sng, int step)
 }
 
 static void
-print_idl_row_simple(const struct idltest_simple *s, int step)
+print_idl_row_simple(const struct idltest_simple *s, int step, bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     ds_put_format(&msg, "i=%"PRId64" r=%g b=%s s=%s u="UUID_FMT" ia=[",
@@ -2042,7 +2047,7 @@ print_idl_row_simple(const struct idltest_simple *s, int 
step)
     }
     ds_put_cstr(&msg, "]");
 
-    char *row_msg = format_idl_row(&s->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&s->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2051,7 +2056,7 @@ print_idl_row_simple(const struct idltest_simple *s, int 
step)
 }
 
 static void
-print_idl_row_link1(const struct idltest_link1 *l1, int step)
+print_idl_row_link1(const struct idltest_link1 *l1, int step, bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     ds_put_format(&msg, "i=%"PRId64" k=", l1->i);
@@ -2070,7 +2075,7 @@ print_idl_row_link1(const struct idltest_link1 *l1, int 
step)
         ds_put_format(&msg, "%"PRId64, l1->l2->i);
     }
 
-    char *row_msg = format_idl_row(&l1->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&l1->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2079,7 +2084,7 @@ print_idl_row_link1(const struct idltest_link1 *l1, int 
step)
 }
 
 static void
-print_idl_row_link2(const struct idltest_link2 *l2, int step)
+print_idl_row_link2(const struct idltest_link2 *l2, int step, bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     ds_put_format(&msg, "i=%"PRId64" l1=", l2->i);
@@ -2087,7 +2092,7 @@ print_idl_row_link2(const struct idltest_link2 *l2, int 
step)
         ds_put_format(&msg, "%"PRId64, l2->l1->i);
     }
 
-    char *row_msg = format_idl_row(&l2->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&l2->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2096,7 +2101,7 @@ print_idl_row_link2(const struct idltest_link2 *l2, int 
step)
 }
 
 static void
-print_idl_row_simple3(const struct idltest_simple3 *s3, int step)
+print_idl_row_simple3(const struct idltest_simple3 *s3, int step, bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     size_t i;
@@ -2115,7 +2120,7 @@ print_idl_row_simple3(const struct idltest_simple3 *s3, 
int step)
     }
     ds_put_cstr(&msg, "]");
 
-    char *row_msg = format_idl_row(&s3->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&s3->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2124,12 +2129,12 @@ print_idl_row_simple3(const struct idltest_simple3 *s3, 
int step)
 }
 
 static void
-print_idl_row_simple4(const struct idltest_simple4 *s4, int step)
+print_idl_row_simple4(const struct idltest_simple4 *s4, int step, bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     ds_put_format(&msg, "name=%s", s4->name);
 
-    char *row_msg = format_idl_row(&s4->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&s4->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2138,7 +2143,7 @@ print_idl_row_simple4(const struct idltest_simple4 *s4, 
int step)
 }
 
 static void
-print_idl_row_simple6(const struct idltest_simple6 *s6, int step)
+print_idl_row_simple6(const struct idltest_simple6 *s6, int step, bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     ds_put_format(&msg, "name=%s ", s6->name);
@@ -2149,7 +2154,7 @@ print_idl_row_simple6(const struct idltest_simple6 *s6, 
int step)
     }
     ds_put_cstr(&msg, "]");
 
-    char *row_msg = format_idl_row(&s6->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&s6->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2158,12 +2163,13 @@ print_idl_row_simple6(const struct idltest_simple6 *s6, 
int step)
 }
 
 static void
-print_idl_row_singleton(const struct idltest_singleton *sng, int step)
+print_idl_row_singleton(const struct idltest_singleton *sng, int step,
+                        bool terse)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
     ds_put_format(&msg, "name=%s", sng->name);
 
-    char *row_msg = format_idl_row(&sng->header_, step, ds_cstr(&msg));
+    char *row_msg = format_idl_row(&sng->header_, step, ds_cstr(&msg), terse);
     print_and_log("%s", row_msg);
     ds_destroy(&msg);
     free(row_msg);
@@ -2172,7 +2178,7 @@ print_idl_row_singleton(const struct idltest_singleton 
*sng, int step)
 }
 
 static void
-print_idl(struct ovsdb_idl *idl, int step)
+print_idl(struct ovsdb_idl *idl, int step, bool terse)
 {
     const struct idltest_simple3 *s3;
     const struct idltest_simple4 *s4;
@@ -2184,31 +2190,31 @@ print_idl(struct ovsdb_idl *idl, int step)
     int n = 0;
 
     IDLTEST_SIMPLE_FOR_EACH (s, idl) {
-        print_idl_row_simple(s, step);
+        print_idl_row_simple(s, step, terse);
         n++;
     }
     IDLTEST_LINK1_FOR_EACH (l1, idl) {
-        print_idl_row_link1(l1, step);
+        print_idl_row_link1(l1, step, terse);
         n++;
     }
     IDLTEST_LINK2_FOR_EACH (l2, idl) {
-        print_idl_row_link2(l2, step);
+        print_idl_row_link2(l2, step, terse);
         n++;
     }
     IDLTEST_SIMPLE3_FOR_EACH (s3, idl) {
-        print_idl_row_simple3(s3, step);
+        print_idl_row_simple3(s3, step, terse);
         n++;
     }
     IDLTEST_SIMPLE4_FOR_EACH (s4, idl) {
-        print_idl_row_simple4(s4, step);
+        print_idl_row_simple4(s4, step, terse);
         n++;
     }
     IDLTEST_SIMPLE6_FOR_EACH (s6, idl) {
-        print_idl_row_simple6(s6, step);
+        print_idl_row_simple6(s6, step, terse);
         n++;
     }
     IDLTEST_SINGLETON_FOR_EACH (sng, idl) {
-        print_idl_row_singleton(sng, step);
+        print_idl_row_singleton(sng, step, terse);
         n++;
     }
     if (!n) {
@@ -2217,7 +2223,7 @@ print_idl(struct ovsdb_idl *idl, int step)
 }
 
 static void
-print_idl_track(struct ovsdb_idl *idl, int step)
+print_idl_track(struct ovsdb_idl *idl, int step, bool terse)
 {
     const struct idltest_simple3 *s3;
     const struct idltest_simple4 *s4;
@@ -2228,27 +2234,27 @@ print_idl_track(struct ovsdb_idl *idl, int step)
     int n = 0;
 
     IDLTEST_SIMPLE_FOR_EACH_TRACKED (s, idl) {
-        print_idl_row_simple(s, step);
+        print_idl_row_simple(s, step, terse);
         n++;
     }
     IDLTEST_LINK1_FOR_EACH_TRACKED (l1, idl) {
-        print_idl_row_link1(l1, step);
+        print_idl_row_link1(l1, step, terse);
         n++;
     }
     IDLTEST_LINK2_FOR_EACH_TRACKED (l2, idl) {
-        print_idl_row_link2(l2, step);
+        print_idl_row_link2(l2, step, terse);
         n++;
     }
     IDLTEST_SIMPLE3_FOR_EACH_TRACKED (s3, idl) {
-        print_idl_row_simple3(s3, step);
+        print_idl_row_simple3(s3, step, terse);
         n++;
     }
     IDLTEST_SIMPLE4_FOR_EACH_TRACKED (s4, idl) {
-        print_idl_row_simple4(s4, step);
+        print_idl_row_simple4(s4, step, terse);
         n++;
     }
     IDLTEST_SIMPLE6_FOR_EACH_TRACKED (s6, idl) {
-        print_idl_row_simple6(s6, step);
+        print_idl_row_simple6(s6, step, terse);
         n++;
     }
 
@@ -2651,6 +2657,13 @@ do_idl(struct ovs_cmdl_context *ctx)
         char *arg = ctx->argv[i];
         struct jsonrpc_msg *request, *reply;
 
+        bool terse = false;
+        if (*arg == '?') {
+            /* We're only interested in terse table contents. */
+            terse = true;
+            arg++;
+        }
+
         if (*arg == '+') {
             /* The previous transaction didn't change anything. */
             arg++;
@@ -2671,10 +2684,10 @@ do_idl(struct ovs_cmdl_context *ctx)
 
             /* Print update. */
             if (track) {
-                print_idl_track(idl, step++);
+                print_idl_track(idl, step++, terse);
                 ovsdb_idl_track_clear(idl);
             } else {
-                print_idl(idl, step++);
+                print_idl(idl, step++, terse);
             }
         }
         seqno = ovsdb_idl_get_seqno(idl);
@@ -2727,7 +2740,7 @@ do_idl(struct ovs_cmdl_context *ctx)
         ovsdb_idl_wait(idl);
         poll_block();
     }
-    print_idl(idl, step++);
+    print_idl(idl, step++, false);
     ovsdb_idl_track_clear(idl);
     ovsdb_idl_destroy(idl);
     print_and_log("%03d: done", step);
@@ -2849,7 +2862,7 @@ dump_simple3(struct ovsdb_idl *idl,
              int step)
 {
     IDLTEST_SIMPLE3_FOR_EACH(myRow, idl) {
-        print_idl_row_simple3(myRow, step);
+        print_idl_row_simple3(myRow, step, false);
     }
 }
 
@@ -2991,7 +3004,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context 
*ctx)
     idltest_simple3_index_set_uref(equal, &myRow2, 1);
     printf("%03d: Query using index with reference\n", step++);
     IDLTEST_SIMPLE3_FOR_EACH_EQUAL (myRow, equal, index) {
-        print_idl_row_simple3(myRow, step++);
+        print_idl_row_simple3(myRow, step++, false);
     }
     idltest_simple3_index_destroy_row(equal);
 
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 5bc0bf681425..853264f22bdc 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -232,75 +232,87 @@ def get_singleton_table_printable_row(row):
     return "name=%s" % row.name
 
 
-def print_row(table, row, step, contents):
-    s = "%03d: table %s: %s " % (step, table, contents)
-    s += get_simple_printable_row_string(row, ["uuid"])
+def print_row(table, row, step, contents, terse):
+    if terse:
+        s = "%03d: table %s" % (step, table)
+    else:
+        s = "%03d: table %s: %s " % (step, table, contents)
+        s += get_simple_printable_row_string(row, ["uuid"])
     print(s)
 
 
-def print_idl(idl, step):
+def print_idl(idl, step, terse=False):
     n = 0
     if "simple" in idl.tables:
         simple = idl.tables["simple"].rows
         for row in simple.values():
             print_row("simple", row, step,
-                      get_simple_table_printable_row(row))
+                      get_simple_table_printable_row(row),
+                      terse)
             n += 1
 
     if "simple2" in idl.tables:
         simple2 = idl.tables["simple2"].rows
         for row in simple2.values():
             print_row("simple2", row, step,
-                      get_simple2_table_printable_row(row))
+                      get_simple2_table_printable_row(row),
+                      terse)
             n += 1
 
     if "simple3" in idl.tables:
         simple3 = idl.tables["simple3"].rows
         for row in simple3.values():
             print_row("simple3", row, step,
-                      get_simple3_table_printable_row(row))
+                      get_simple3_table_printable_row(row),
+                      terse)
             n += 1
 
     if "simple4" in idl.tables:
         simple4 = idl.tables["simple4"].rows
         for row in simple4.values():
             print_row("simple4", row, step,
-                      get_simple4_table_printable_row(row))
+                      get_simple4_table_printable_row(row),
+                      terse)
             n += 1
 
     if "simple5" in idl.tables:
         simple5 = idl.tables["simple5"].rows
         for row in simple5.values():
             print_row("simple5", row, step,
-                      get_simple5_table_printable_row(row))
+                      get_simple5_table_printable_row(row),
+                      terse)
             n += 1
 
     if "simple6" in idl.tables:
         simple6 = idl.tables["simple6"].rows
         for row in simple6.values():
             print_row("simple6", row, step,
-                      get_simple6_table_printable_row(row))
+                      get_simple6_table_printable_row(row),
+                      terse)
             n += 1
 
     if "link1" in idl.tables:
         l1 = idl.tables["link1"].rows
         for row in l1.values():
             print_row("link1", row, step,
-                      get_link1_table_printable_row(row))
+                      get_link1_table_printable_row(row),
+                      terse)
             n += 1
 
     if "link2" in idl.tables:
         l2 = idl.tables["link2"].rows
         for row in l2.values():
             print_row("link2", row, step,
-                      get_link2_table_printable_row(row))
+                      get_link2_table_printable_row(row),
+                      terse)
             n += 1
 
     if "singleton" in idl.tables:
         sng = idl.tables["singleton"].rows
         for row in sng.values():
             print_row("singleton", row, step,
-                      get_singleton_table_printable_row(row))
+                      get_singleton_table_printable_row(row),
+                      terse)
             n += 1
 
     if not n:
@@ -701,6 +713,12 @@ def do_idl(schema_file, remote, *commands):
         step += 1
 
     for command in commands:
+        terse = False
+        if command.startswith("?"):
+            # We're only interested in terse table contents.
+            terse = True
+            command = command[1:]
+
         if command.startswith("+"):
             # The previous transaction didn't change anything.
             command = command[1:]
@@ -714,7 +732,7 @@ def do_idl(schema_file, remote, *commands):
                 rpc.wait(poller)
                 poller.block()
 
-            print_idl(idl, step)
+            print_idl(idl, step, terse)
             step += 1
 
         seqno = idl.change_seqno

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to