ovsdb-cs: Avoid unnecessary re-connections when updating remotes.

If a new database server added to the cluster, or if one of the
database servers changed its IP address or port, then you need to
update the list of remotes for the client.  For example, if a new
OVN_Southbound database server is added, you need to update the
ovn-remote for the ovn-controller.

However, in the current implementation, the ovsdb-cs module always
closes the current connection and creates a new one.  This can lead
to a storm of re-connections if all ovn-controllers will be updated
simultaneously.  They can also start re-dowloading the database
content, creating even more load on the database servers.

Correct this by saving an existing connection if it is still in the
list of remotes after the update.

'reconnect' module will report connection state updates, but that
is OK since no real re-connection happened and we only updated the
state of a new 'reconnect' instance.

If required, re-connection can be forced after the update of remotes
with ovsdb_cs_force_reconnect().

Acked-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

lib/jsonrpc.c

@@ -952,6 +952,19 @@ jsonrpc_session_steal(struct jsonrpc_session *s)
     return rpc;
 }
 
+void
+jsonrpc_session_replace(struct jsonrpc_session *s, struct jsonrpc *rpc)
+{
+    if (s->rpc) {
+        jsonrpc_close(s->rpc);
+    }
+    s->rpc = rpc;
+    if (s->rpc) {
+        reconnect_set_name(s->reconnect, jsonrpc_get_name(s->rpc));
+        reconnect_connected(s->reconnect, time_msec());
+    }
+}
+
 static void
 jsonrpc_session_disconnect(struct jsonrpc_session *s)
 {

lib/jsonrpc.h

@@ -114,6 +114,7 @@ struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *,
 void jsonrpc_session_close(struct jsonrpc_session *);
 
 struct jsonrpc *jsonrpc_session_steal(struct jsonrpc_session *);
+void jsonrpc_session_replace(struct jsonrpc_session *, struct jsonrpc *);
 
 void jsonrpc_session_run(struct jsonrpc_session *);
 void jsonrpc_session_wait(struct jsonrpc_session *);

lib/ovsdb-cs.c

@@ -660,7 +660,8 @@ ovsdb_cs_wait(struct ovsdb_cs *cs)
 
 /* Network connection. */
 
-/* Changes the remote and creates a new session.
+/* Changes the remote and creates a new session.  Keeps existing connection
+ * if current remote is still valid.
  *
  * If 'retry' is true, the connection to the remote will automatically retry
  * when it fails.  If 'retry' is false, the connection is one-time. */
@@ -670,9 +671,12 @@ ovsdb_cs_set_remote(struct ovsdb_cs *cs, const char *remote, bool retry)
     if (cs
         && ((remote != NULL) != (cs->remote != NULL)
             || (remote && cs->remote && strcmp(remote, cs->remote)))) {
+        struct jsonrpc *rpc = NULL;
+
         /* Close the old session, if any. */
         if (cs->session) {
-            jsonrpc_session_close(cs->session);
+            /* Save the current open connection and close the session. */
+            rpc = jsonrpc_session_steal(cs->session);
             cs->session = NULL;
 
             free(cs->remote);
@@ -682,17 +686,30 @@ ovsdb_cs_set_remote(struct ovsdb_cs *cs, const char *remote, bool retry)
         /* Open new session, if any. */
         if (remote) {
             struct svec remotes = SVEC_EMPTY_INITIALIZER;
+            struct uuid old_cid = cs->cid;
+
             ovsdb_session_parse_remote(remote, &remotes, &cs->cid);
             if (cs->shuffle_remotes) {
                 svec_shuffle(&remotes);
             }
             cs->session = jsonrpc_session_open_multiple(&remotes, retry);
+
+            /* Use old connection, if cluster id didn't change and the remote
+             * is still on the list, to avoid unnecessary re-connection. */
+            if (rpc && uuid_equals(&old_cid, &cs->cid)
+                && svec_contains_unsorted(&remotes, jsonrpc_get_name(rpc))) {
+                jsonrpc_session_replace(cs->session, rpc);
+                cs->state_seqno = jsonrpc_session_get_seqno(cs->session);
+                rpc = NULL;
+            } else {
+                cs->state_seqno = UINT_MAX;
+            }
+
             svec_destroy(&remotes);
-
-            cs->state_seqno = UINT_MAX;
-
             cs->remote = xstrdup(remote);
         }
+
+        jsonrpc_close(rpc);
     }
 }
 

lib/svec.c

@@ -247,6 +247,17 @@ svec_contains(const struct svec *svec, const char *name)
     return svec_find(svec, name) != SIZE_MAX;
 }
 
+bool
+svec_contains_unsorted(const struct svec *svec, const char *name)
+{
+    for (size_t i = 0; i < svec->n; i++) {
+        if (!strcmp(svec->names[i], name)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 size_t
 svec_find(const struct svec *svec, const char *name)
 {

lib/svec.h

@@ -50,6 +50,7 @@ void svec_shuffle(struct svec *);
 void svec_diff(const struct svec *a, const struct svec *b,
                struct svec *a_only, struct svec *both, struct svec *b_only);
 bool svec_contains(const struct svec *, const char *);
+bool svec_contains_unsorted(const struct svec *, const char *);
 size_t svec_find(const struct svec *, const char *);
 bool svec_is_sorted(const struct svec *);
 bool svec_is_unique(const struct svec *);

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
+]])

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++);