[ovs-dev] [PATCH] ovsdb-cs: Avoid unnecessary re-connections when updating remotes.

[Ilya Maximets]

On 6/29/21 9:57 PM, Ilya Maximets wrote:
>>> Regarding the current patch, I think it's better to add a test case to
>>> cover the scenario and confirm that existing connections didn't reset. With
>>> that:
>>> Acked-by: Han Zhou <hzhou at ovn.org>
> 
> I'll work on a unit test for this.

Hi.  Here is a unit test that I came up with:

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 62181dd4d..e32f9ec89 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2282,3 +2282,27 @@ OVSDB_CHECK_CLUSTER_IDL_C([simple idl, monitor_cond_since, cluster disconnect],
 008: table simple: i=1 r=2 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2>
 009: done
 ]])
+
+dnl This test checks that IDL keeps the existing connection to the server if
+dnl it's still on a list of remotes after update.
+OVSDB_CHECK_IDL_C([simple idl, initially empty, set remotes],
+  [],
+  [['set-remote unix:socket' \
+    '+set-remote unix:bad_socket,unix:socket' \
+    '+set-remote unix:bad_socket' \
+    '+set-remote unix:socket' \
+    'set-remote unix:bad_socket,unix:socket' \
+    '+set-remote unix:socket' \
+    '+reconnect']],
+  [[000: empty
+001: new remotes: unix:socket, is connected: true
+002: new remotes: unix:bad_socket,unix:socket, is connected: true
+003: new remotes: unix:bad_socket, is connected: false
+004: new remotes: unix:socket, is connected: false
+005: empty
+006: new remotes: unix:bad_socket,unix:socket, is connected: true
+007: new remotes: unix:socket, is connected: true
+008: reconnect
+009: empty
+010: done
+]])
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index a886f971e..93329cd4c 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2621,6 +2621,7 @@ do_idl(struct ovs_cmdl_context *ctx)
     setvbuf(stdout, NULL, _IONBF, 0);
 
     symtab = ovsdb_symbol_table_create();
+    const char remote_s[] = "set-remote ";
     const char cond_s[] = "condition ";
     if (ctx->argc > 2 && strstr(ctx->argv[2], cond_s)) {
         update_conditions(idl, ctx->argv[2] + strlen(cond_s));
@@ -2664,6 +2665,11 @@ do_idl(struct ovs_cmdl_context *ctx)
         if (!strcmp(arg, "reconnect")) {
             print_and_log("%03d: reconnect", step++);
             ovsdb_idl_force_reconnect(idl);
+        }  else if (!strncmp(arg, remote_s, strlen(remote_s))) {
+            ovsdb_idl_set_remote(idl, arg + strlen(remote_s), true);
+            print_and_log("%03d: new remotes: %s, is connected: %s", step++,
+                          arg + strlen(remote_s),
+                          ovsdb_idl_is_connected(idl) ? "true" : "false");
         }  else if (!strncmp(arg, cond_s, strlen(cond_s))) {
             update_conditions(idl, arg + strlen(cond_s));
             print_and_log("%03d: change conditions", step++);
---

Dumitru, Han, if it looks good to you, I can squash it in before
applying the patch.   What do you think?

Best regards, Ilya Maximets.