Patch 1/2 of this patchset removed the pin's most visible symptom:
hundreds of poll passes a second. A peer that has stopped reading is
now entirely silent -- still never probed, never disconnected, nothing
logged, and no longer costing the CPU that used to give it away.
Let the ordinary probe and timeout run for such a session instead. A
session whose peer appears to take nothing for two probe intervals is
terminated, and says so through the existing "no response to inactivity
probe" message, extended to name the bytes still queued.
That rule is already written down in raft_conn_update_probe_interval()
-- "Connection will be dropped if inactivity will last twice that time"
-- and a stalled raft connection does not reach it either today; only
raft's 500 message backlog cap eventually does. This makes the
documented rule hold for every jsonrpc session, raft's included.
What matters is not how fast a peer reads but whether the server can see
it reading. All the server sees is its own queue shrinking, and that
happens only when the socket send buffer has room -- which the peer
controls, by acknowledging data already sent and by reopening its
receive window as it reads. A peer that moves the queue within the
interval is unaffected, however slowly it consumes; a peer that moves
nothing is indistinguishable from one that stopped, and is now treated
as one. On this host, at the 5 s default inactivity_probe, that line
falls between 10 and 13 kB/s, and it scales inversely with
inactivity_probe. Raising that setting is how a deployment gives its
consumers longer to become visible.
Testing. Applied on top of patch 1/2 of this patchset, whose CPU fix
this does not touch; what changes is which sessions survive. At the 5 s
TCP default:
client without this patch with it
stops reading kept dropped at t+12.1 s
reads 4 kB/s kept dropped at t+28 s
reads 20 kB/s kept kept
Signed-off-by: Aeliton G. Silva <[email protected]>
Assisted-by: Claude Opus 5, Claude Code
---
lib/reconnect.c | 31 ++++++++------
python/ovs/reconnect.py | 31 ++++++++------
tests/reconnect.at | 92 ++++++++++++++++++++++++++++++++++++-----
3 files changed, 119 insertions(+), 35 deletions(-)
diff --git a/lib/reconnect.c b/lib/reconnect.c
index 1f9bf83df..f033cecf6 100644
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -500,9 +500,10 @@ reconnect_connect_failed(struct reconnect *fsm, long long
int now, int error)
* the probe interval timer, so that the connection is known not to be idle.
*
* 'queued_bytes' is data queued for the peer that could not be sent. While
- * it is nonzero the FSM stops asking to be woken up to attempt a receive:
- * the caller evidently cannot get data to this peer, so no receive it makes
- * can settle anything, and waking to try only burns CPU. */
+ * it is nonzero the probe interval is allowed to expire on its own schedule
+ * rather than asking for a fast wake-up to attempt a receive: the caller
+ * evidently cannot get data to this peer, so no receive it makes can settle
+ * anything, and the ordinary probe and timeout should run their course. */
void
reconnect_activity(struct reconnect *fsm, long long int now,
size_t queued_bytes)
@@ -566,13 +567,10 @@ reconnect_deadline__(const struct reconnect *fsm, long
long int now)
case S_ACTIVE:
if (fsm->probe_interval) {
- if (fsm->queued_bytes) {
- return LLONG_MAX;
- }
-
long long int base = MAX(fsm->last_activity, fsm->state_entered);
long long int expiration = base + fsm->probe_interval;
- if (now < expiration || fsm->last_receive_attempt >= expiration) {
+ if (now < expiration || fsm->last_receive_attempt >= expiration
+ || fsm->queued_bytes) {
/* We still have time before the expiration or the time has
* already passed and there was no activity. In the first case
* we need to wait for the expiration, in the second - we're
@@ -591,7 +589,8 @@ reconnect_deadline__(const struct reconnect *fsm, long long
int now)
case S_IDLE:
if (fsm->probe_interval) {
long long int expiration = fsm->state_entered +
fsm->probe_interval;
- if (now < expiration || fsm->last_receive_attempt >= expiration) {
+ if (now < expiration || fsm->last_receive_attempt >= expiration
+ || fsm->queued_bytes) {
return expiration;
} else {
return now + 1;
@@ -663,9 +662,17 @@ reconnect_run(struct reconnect *fsm, long long int now)
return RECONNECT_PROBE;
case S_IDLE:
- VLOG_ERR("%s: no response to inactivity probe after %.3g "
- "seconds, disconnecting",
- fsm->name, (now - fsm->state_entered) / 1000.0);
+ if (fsm->queued_bytes) {
+ VLOG_ERR("%s: no response to inactivity probe after %.3g "
+ "seconds, with %"PRIuSIZE" bytes still queued for "
+ "the peer, disconnecting", fsm->name,
+ (now - fsm->state_entered) / 1000.0,
+ fsm->queued_bytes);
+ } else {
+ VLOG_ERR("%s: no response to inactivity probe after %.3g "
+ "seconds, disconnecting",
+ fsm->name, (now - fsm->state_entered) / 1000.0);
+ }
return RECONNECT_DISCONNECT;
case S_RECONNECT:
diff --git a/python/ovs/reconnect.py b/python/ovs/reconnect.py
index 2ff4fc6c4..382c1dbf1 100644
--- a/python/ovs/reconnect.py
+++ b/python/ovs/reconnect.py
@@ -94,14 +94,12 @@ class Reconnect(object):
@staticmethod
def deadline(fsm, now):
if fsm.probe_interval:
- if fsm.queued_bytes:
- return None
-
base = max(fsm.last_activity, fsm.state_entered)
expiration = base + fsm.probe_interval
if (now < expiration or
fsm.last_receive_attempt is None or
- fsm.last_receive_attempt >= expiration):
+ fsm.last_receive_attempt >= expiration or
+ fsm.queued_bytes):
# We still have time before the expiration or the time has
# already passed and there was no activity. In the first
# case we need to wait for the expiration, in the second -
@@ -133,7 +131,8 @@ class Reconnect(object):
expiration = fsm.state_entered + fsm.probe_interval
if (now < expiration or
fsm.last_receive_attempt is None or
- fsm.last_receive_attempt >= expiration):
+ fsm.last_receive_attempt >= expiration or
+ fsm.queued_bytes):
return expiration
else:
return now + 1
@@ -141,9 +140,16 @@ class Reconnect(object):
@staticmethod
def run(fsm, now):
- vlog.err("%s: no response to inactivity probe after %.3g "
- "seconds, disconnecting"
- % (fsm.name, (now - fsm.state_entered) / 1000.0))
+ if fsm.queued_bytes:
+ vlog.err("%s: no response to inactivity probe after %.3g "
+ "seconds, with %d bytes still queued for the peer, "
+ "disconnecting"
+ % (fsm.name, (now - fsm.state_entered) / 1000.0,
+ fsm.queued_bytes))
+ else:
+ vlog.err("%s: no response to inactivity probe after %.3g "
+ "seconds, disconnecting"
+ % (fsm.name, (now - fsm.state_entered) / 1000.0))
return DISCONNECT
class Reconnect(object):
@@ -496,10 +502,11 @@ class Reconnect(object):
not to be idle.
'queued_bytes' is data queued for the peer that could not be sent.
- While it is nonzero the FSM stops asking to be woken up to attempt a
- receive: the caller evidently cannot get data to this peer, so no
- receive it makes can settle anything, and waking to try only burns
- CPU."""
+ While it is nonzero the probe interval is allowed to expire on its own
+ schedule rather than asking for a fast wake-up to attempt a receive:
+ the caller evidently cannot get data to this peer, so no receive it
+ makes can settle anything, and the ordinary probe and timeout should
+ run their course."""
self.queued_bytes = queued_bytes
if self.state != Reconnect.Active:
self._transition(now, Reconnect.Active)
diff --git a/tests/reconnect.at b/tests/reconnect.at
index 650ca3485..d6fd3ccf1 100644
--- a/tests/reconnect.at
+++ b/tests/reconnect.at
@@ -1365,7 +1365,7 @@ listening
])
######################################################################
-RECONNECT_CHECK([no wake-up while data is queued],
+RECONNECT_CHECK([peer with queued data is dropped],
[enable
# Connection succeeds.
@@ -1375,9 +1375,12 @@ connected
# Data is queued for the peer that we could not send.
activity 1000
-# Long past the probe interval the FSM asks for no wake-up at all, and
-# nothing happens: the connection is neither probed nor disconnected.
-advance 60000
+# Past the probe interval the connection is probed, on the ordinary
+# schedule rather than after a fast wake-up.
+timeout
+run
+
+# And a probe interval later, with nothing taken, it is given up on.
timeout
run
],
@@ -1398,15 +1401,25 @@ connected
# Data is queued for the peer that we could not send.
activity 1000
-# Long past the probe interval the FSM asks for no wake-up at all, and
-# nothing happens: the connection is neither probed nor disconnected.
-advance 60000
+# Past the probe interval the connection is probed, on the ordinary
+# schedule rather than after a fast wake-up.
+timeout
+ advance 5000 ms
+
+### t=6000 ###
+ in ACTIVE for 5000 ms (0 ms backoff)
+run
+ should send probe
+ in IDLE for 0 ms (0 ms backoff)
-### t=61000 ###
- in ACTIVE for 60000 ms (0 ms backoff)
+# And a probe interval later, with nothing taken, it is given up on.
timeout
- no timeout
+ advance 5000 ms
+
+### t=11000 ###
+ in IDLE for 5000 ms (0 ms backoff)
run
+ should disconnect
])
######################################################################
@@ -1445,16 +1458,19 @@ advance 10000
### t=11000 ###
in ACTIVE for 10000 ms (0 ms backoff)
run
+ should send probe
+ in IDLE for 0 ms (0 ms backoff)
# The queue drains, so the ordinary probe interval applies again.
activity 0
+ in ACTIVE for 0 ms (0 ms backoff)
created 1000, last activity 11000, last connected 1000
receive-attempted LLONG_MAX
timeout
advance 5000 ms
### t=16000 ###
- in ACTIVE for 15000 ms (0 ms backoff)
+ in ACTIVE for 5000 ms (0 ms backoff)
run
should send probe
in IDLE for 0 ms (0 ms backoff)
@@ -1507,3 +1523,57 @@ timeout
### t=6000 ###
in ACTIVE for 5000 ms (1000 ms backoff)
])
+
+######################################################################
+RECONNECT_CHECK([peer taking data is kept],
+ [enable
+run
+connected
+
+# A peer that keeps taking data is kept, however much is still queued:
+# every drain moves the expiration out, so the probe never comes due.
+activity 1000
+advance 4000
+run
+activity 500
+advance 4000
+run
+activity 200
+advance 4000
+run
+],
+ [### t=1000 ###
+enable
+ in BACKOFF for 0 ms (0 ms backoff)
+run
+ should connect
+connected
+ in ACTIVE for 0 ms (0 ms backoff)
+ created 1000, last activity 1000, last connected 1000
+ 1 successful connections out of 1 attempts, seqno 1
+ connected
+ last connected 0 ms ago, connected 0 ms total
+
+# A peer that keeps taking data is kept, however much is still queued:
+# every drain moves the expiration out, so the probe never comes due.
+activity 1000
+advance 4000
+
+### t=5000 ###
+ in ACTIVE for 4000 ms (0 ms backoff)
+run
+activity 500
+ created 1000, last activity 5000, last connected 1000
+advance 4000
+
+### t=9000 ###
+ in ACTIVE for 8000 ms (0 ms backoff)
+run
+activity 200
+ created 1000, last activity 9000, last connected 1000
+advance 4000
+
+### t=13000 ###
+ in ACTIVE for 12000 ms (0 ms backoff)
+run
+])
--
2.43.0
--
_'Esta mensagem é direcionada apenas para os endereços constantes no
cabeçalho inicial. Se você não está listado nos endereços constantes no
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão
imediatamente anuladas e proibidas'._
* **'Apesar do Magazine Luiza tomar
todas as precauções razoáveis para assegurar que nenhum vírus esteja
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.*
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
