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>

5 files changed, 47 insertions, 4 deletions

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 926cbcb86..7e333ae25 100644
--- a/lib/jsonrpc.c
+++ b/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)
 {
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index d75d66b86..034a30b71 100644
--- a/lib/jsonrpc.h
+++ b/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 *);
diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index 911b71dd4..22630955d 100644
--- a/lib/ovsdb-cs.c
+++ b/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);
-            svec_destroy(&remotes);
 
-            cs->state_seqno = UINT_MAX;
+            /* 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->remote = xstrdup(remote);
         }
+
+        jsonrpc_close(rpc);
     }
 }
 
diff --git a/lib/svec.c b/lib/svec.c
index c1b986bab..6ea96384b 100644
--- a/lib/svec.c
+++ b/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)
 {
diff --git a/lib/svec.h b/lib/svec.h
index b4e1343a9..cd95871cc 100644
--- a/lib/svec.h
+++ b/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 *);