[ovs-dev] [PATCH v2] jsonrpc: Add a new API to allow clients to reset reconnect backoff.
Dumitru Ceara
dceara at redhat.com
Wed Jul 21 10:56:13 UTC 2021
Add jsonrpc_session_reset_backoff() which resets the number of
backoff-free tries to the number of remotes + 1. This is used by
upper layers (e.g., ovsdb-cs) before forcefully triggering a
reconnection. Like this the reconnection will happen as soon as
possible.
The backoff mechanism should be used whenever the remote is overloaded
and causes connection drops. However, forceful reconnects triggered by
upper layers happen due to other reasons, e.g.: inconsistent data
received from the remote, the remote becomes a follower and the client
needs a "leader-only" connection.
Since 3c2d6274bcee ("raft: Transfer leadership before creating
snapshots."), leadership changes inside the clustered database happen
more often and therefore "leader-only" clients need to reconnect more
often too. Not resetting the backoff every time a leadership change
happens will cause all reconnections to happen with the maximum backoff
(8 seconds) resulting in significant latency.
Use jsonrpc_session_reset_backoff() in ovsdb-cs when forcing reconnects,
if the session is in state CS_S_MONITORING. This commit also updates
the Python IDL and jsonrpc implementations. The Python IDL wasn't
tracking the IDL_S_MONITORING state explicitly, we now do that too.
This commit also adds tests, using test-ovsdb(.py), making sure the IDL
forced reconnects happen without backoff.
Reported-at: https://bugzilla.redhat.com/1977264
Suggested-by: Ilya Maximets <i.maximets at ovn.org>
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
The original v1 patch which was trying to add a new type of reconnect
operation was posted here:
https://patchwork.ozlabs.org/project/openvswitch/patch/20210629112035.17402-1-dceara@redhat.com/
v2:
- Reworked the patch based on the discussion with Ilya:
- add a new jsonrpc_session_reset_backoff() API to be used by the
ovsdb-cs layer to avoid backoffs when reconnecting due to
inconsistent data/remote becoming follower.
---
lib/jsonrpc.c | 14 +++++++
lib/jsonrpc.h | 1 +
lib/ovsdb-cs.c | 10 +++++
python/ovs/db/idl.py | 11 +++++
python/ovs/jsonrpc.py | 13 ++++++
tests/ovsdb-idl.at | 93 +++++++++++++++++++++++++++++++++++++++++--
6 files changed, 138 insertions(+), 4 deletions(-)
diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 7e333ae25f..4dd0b4b2b0 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1280,6 +1280,20 @@ jsonrpc_session_force_reconnect(struct jsonrpc_session *s)
reconnect_force_reconnect(s->reconnect, time_msec());
}
+/* Resets the reconnect backoff for 's' by allowing as many free tries as the
+ * number of configured remotes + 1. This is to be used by upper layers before
+ * calling jsonrpc_session_force_reconnect() when one of the other, not
+ * currently connected, remotes should be used instead (e.g., based on the
+ * contents of the data received from the currently connected remote).
+ *
+ * The "+1" free try will be consumed when the current remote is disconnected.
+ */
+void
+jsonrpc_session_reset_backoff(struct jsonrpc_session *s)
+{
+ reconnect_set_backoff_free_tries(s->reconnect, s->remotes.n + 1);
+}
+
/* Sets 'max_backoff' as the maximum time, in milliseconds, to wait after a
* connection attempt fails before attempting to connect again. */
void
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index 034a30b716..2aa97d3fe6 100644
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -137,6 +137,7 @@ void jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *,
void jsonrpc_session_enable_reconnect(struct jsonrpc_session *);
void jsonrpc_session_force_reconnect(struct jsonrpc_session *);
+void jsonrpc_session_reset_backoff(struct jsonrpc_session *);
void jsonrpc_session_set_max_backoff(struct jsonrpc_session *,
int max_backoff);
diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index f13065c6c0..659d49dbf7 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -729,6 +729,16 @@ void
ovsdb_cs_force_reconnect(struct ovsdb_cs *cs)
{
if (cs->session) {
+ if (cs->state == CS_S_MONITORING) {
+ /* The ovsdb-cs was in MONITORING state, so we either had data
+ * inconsistency on this server, or it stopped being the cluster
+ * leader, or the user requested to re-connect. Avoiding backoff
+ * in these cases, as we need to re-connect as soon as possible.
+ * Connections that are not in MONITORING state should have their
+ * backoff to avoid constant flood of re-connection attempts in
+ * case there is no suitable database server. */
+ jsonrpc_session_reset_backoff(cs->session);
+ }
jsonrpc_session_force_reconnect(cs->session);
}
}
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index f24511720f..ecae5e1432 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -102,6 +102,7 @@ class Idl(object):
IDL_S_SERVER_MONITOR_REQUESTED = 2
IDL_S_DATA_MONITOR_REQUESTED = 3
IDL_S_DATA_MONITOR_COND_REQUESTED = 4
+ IDL_S_MONITORING = 5
def __init__(self, remote, schema_helper, probe_interval=None,
leader_only=True):
@@ -295,6 +296,7 @@ class Idl(object):
else:
assert self.state == self.IDL_S_DATA_MONITOR_REQUESTED
self.__parse_update(msg.result, OVSDB_UPDATE)
+ self.state = self.IDL_S_MONITORING
except error.Error as e:
vlog.err("%s: parse error in received schema: %s"
@@ -442,6 +444,15 @@ class Idl(object):
def force_reconnect(self):
"""Forces the IDL to drop its connection to the database and reconnect.
In the meantime, the contents of the IDL will not change."""
+ if self.state == self.IDL_S_MONITORING:
+ # The IDL was in MONITORING state, so we either had data
+ # inconsistency on this server, or it stopped being the cluster
+ # leader, or the user requested to re-connect. Avoiding backoff
+ # in these cases, as we need to re-connect as soon as possible.
+ # Connections that are not in MONITORING state should have their
+ # backoff to avoid constant flood of re-connection attempts in
+ # case there is no suitable database server.
+ self._session.reset_backoff()
self._session.force_reconnect()
def session_name(self):
diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
index bf32f8c87c..5b8daa91ec 100644
--- a/python/ovs/jsonrpc.py
+++ b/python/ovs/jsonrpc.py
@@ -612,5 +612,18 @@ class Session(object):
def force_reconnect(self):
self.reconnect.force_reconnect(ovs.timeval.msec())
+ def reset_backoff(self):
+ """ Resets the reconnect backoff by allowing as many free tries as the
+ number of configured remotes + 1. This is to be used by upper
+ layers before calling force_reconnect() when one of the other, not
+ currently connected, remotes should be used instead (e.g., based on
+ the contents of the data received from the currently connected
+ remote).
+
+ The "+1" free try will be consumed when the current remote is
+ disconnected."""
+
+ self.reconnect.set_backoff_free_tries(len(self.remotes) + 1)
+
def get_num_of_remotes(self):
return len(self.remotes)
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index e32f9ec895..a01f7bbd58 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2227,11 +2227,29 @@ m4_define([OVSDB_CHECK_IDL_LEADER_ONLY_PY],
OVSDB_CHECK_IDL_LEADER_ONLY_PY([Check Python IDL connects to leader], 3, ['remote'])
OVSDB_CHECK_IDL_LEADER_ONLY_PY([Check Python IDL reconnects to leader], 3, ['remote' '+remotestop' 'remote'])
-# same as OVSDB_CHECK_IDL but uses C IDL implementation with tcp
-# with multiple remotes.
+# OVSDB_CHECK_CLUSTER_IDL_C(TITLE, N_SERVERS, [PRE-IDL-TXN], TRANSACTIONS,
+# OUTPUT, [KEYWORDS], [FILTER], [LOG_FILTER])
+#
+# Creates a clustered database with a schema derived from idltest.ovsidl, runs
+# each PRE-IDL-TXN (if any), starts N_SERVERS ovsdb-server instances in RAFT,
+# on that database, and runs "test-ovsdb idl" passing each of the TRANSACTIONS
+# along.
+#
+# Checks that the overall output is OUTPUT. Before comparison, the
+# output is sorted (using "sort") and UUIDs in the output are replaced
+# by markers of the form <N> where N is a number. The first unique
+# UUID is replaced by <0>, the next by <1>, and so on. If a given
+# UUID appears more than once it is always replaced by the same
+# marker. If FILTER is supplied then the output is also filtered
+# through the specified program.
+#
+# TITLE is provided to AT_SETUP and KEYWORDS to AT_KEYWORDS.
+#
+# If LOG_FILTER is provided, checks that the contents of LOG_FILTER
+# are not matched by grep in the test-ovsdb logs.
m4_define([OVSDB_CHECK_CLUSTER_IDL_C],
[AT_SETUP([$1 - C - tcp])
- AT_KEYWORDS([ovsdb server idl positive tcp socket $5])
+ AT_KEYWORDS([ovsdb server idl tcp $6])
m4_define([LPBK],[127.0.0.1])
OVSDB_CLUSTER_START_IDLTEST([$2], ["ptcp:0:"LPBK])
PARSE_LISTENING_PORT([s1.log], [TCP_PORT_1])
@@ -2242,11 +2260,55 @@ m4_define([OVSDB_CHECK_CLUSTER_IDL_C],
m4_if([$3], [], [],
[AT_CHECK([ovsdb-client transact $remotes $3], [0], [ignore], [ignore])])
AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 idl tcp:LPBK:$TCP_PORT_1 $4],
- [0], [stdout], [ignore])
+ [0], [stdout], [stderr])
+ AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]),
+ [0], [$5])
+ m4_ifval([$8], [AT_CHECK([grep '$8' stderr], [1])], [], [])
+ AT_CLEANUP])
+
+# OVSDB_CHECK_CLUSTER_IDL_C(TITLE, N_SERVERS, [PRE-IDL-TXN], TRANSACTIONS,
+# OUTPUT, [KEYWORDS], [FILTER], [LOG_FILTER])
+#
+# Creates a clustered database with a schema derived from idltest.ovsidl, runs
+# each PRE-IDL-TXN (if any), starts N_SERVERS ovsdb-server instances in RAFT,
+# on that database, and runs "test-ovsdb.py idl" passing each of the
+# TRANSACTIONS along.
+#
+# Checks that the overall output is OUTPUT. Before comparison, the
+# output is sorted (using "sort") and UUIDs in the output are replaced
+# by markers of the form <N> where N is a number. The first unique
+# UUID is replaced by <0>, the next by <1>, and so on. If a given
+# UUID appears more than once it is always replaced by the same
+# marker. If FILTER is supplied then the output is also filtered
+# through the specified program.
+#
+# TITLE is provided to AT_SETUP and KEYWORDS to AT_KEYWORDS.
+#
+# If LOG_FILTER is provided, checks that the contents of LOG_FILTER
+# are not matched by grep in the test-ovsdb.py logs.
+m4_define([OVSDB_CHECK_CLUSTER_IDL_PY],
+ [AT_SETUP([$1 - Python3 - tcp])
+ AT_KEYWORDS([ovsdb server idl tcp $6])
+ m4_define([LPBK],[127.0.0.1])
+ OVSDB_CLUSTER_START_IDLTEST([$2], ["ptcp:0:"LPBK])
+ PARSE_LISTENING_PORT([s1.log], [TCP_PORT_1])
+ PARSE_LISTENING_PORT([s2.log], [TCP_PORT_2])
+ PARSE_LISTENING_PORT([s3.log], [TCP_PORT_3])
+ remotes=tcp:LPBK:$TCP_PORT_1,tcp:LPBK:$TCP_PORT_2,tcp:LPBK:$TCP_PORT_3
+
+ m4_if([$3], [], [],
+ [AT_CHECK([ovsdb-client transact $remotes $3], [0], [ignore], [ignore])])
+ AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t10 idl $srcdir/idltest.ovsschema tcp:LPBK:$TCP_PORT_1 $4],
+ [0], [stdout], [stderr])
AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]),
[0], [$5])
+ m4_if([$8], [AT_CHECK([grep '$8' stderr], [1])], [], [])
AT_CLEANUP])
+m4_define([OVSDB_CHECK_CLUSTER_IDL],
+ [OVSDB_CHECK_CLUSTER_IDL_C($@)
+ OVSDB_CHECK_CLUSTER_IDL_PY($@)])
+
# Checks that monitor_cond_since works fine when disconnects happen
# with cond_change requests in flight (i.e., IDL is properly updated).
OVSDB_CHECK_CLUSTER_IDL_C([simple idl, monitor_cond_since, cluster disconnect],
@@ -2306,3 +2368,26 @@ OVSDB_CHECK_IDL_C([simple idl, initially empty, set remotes],
009: empty
010: done
]])
+
+dnl This test checks that forceful reconnects triggered by the IDL
+dnl happen immediately (they should not use backoff).
+OVSDB_CHECK_CLUSTER_IDL([simple idl, initially empty, force reconnect],
+ 3,
+ [],
+ [['+reconnect' \
+ 'reconnect' \
+ 'reconnect' \
+ 'reconnect']],
+ [[000: reconnect
+001: empty
+002: reconnect
+003: empty
+004: reconnect
+005: empty
+006: reconnect
+007: empty
+008: done
+]],
+[],
+[],
+reconnect.*waiting .* seconds before reconnect)
--
2.27.0
More information about the dev
mailing list