Thanks for looking at this.
> If we're actually never calling the recv() on a connection, that is
> likely what we should be addressing instead of inventing workarounds.
> And this should natuarally disconnect clients that do not respond to
> inactivity probes.
I did consider that, and even tried it out: with the receive ungated a
wedged peer is dropped by the inactivity timeout, and make check still
passes. But I still think it might not be the best solution, at least
not at this moment, given the pros and cons I have weighed.
The main argument is the change of behaviour -- and note that a
backlogged session cannot answer a probe in any case, since the probe is
queued behind the backlog: an answer would prove that the queue drained,
not that the peer is alive. Currently a session that does not clear its
backlog within an inactivity probe interval is ignored and left alone,
so it can eventually finish reading its backlog, and then it starts
receiving and answering probes again. This is not documented anywhere I
could find, but it looks intentional: ovsdb_jsonrpc_monitor_flush_all()
sits inside the same gate, placed there by 48f6e410d right after
633f72479 reverted 60533a405 ("jsonrpc-server: Disconnect connections
that queue too much data.").
If we change that and start attempting a receive on such sessions, they
begin to be dropped where they previously were not, and a client that
cannot drain in time is dropped again after each fresh start -- a
disconnect-reconnect loop the current implementation does not give us
for the same clients. That is precisely why 60533a405 was reverted:
"this is not ideal because of situations where disconnection happens
repeatedly." So it smells to me like a change set up to be reverted the
same way.
The proposed change does the following (in short):
1/2: preserve behaviour, do not spin CPU on backlogged sessions;
2/2: treat a session that has been seen reading as active and drop
**only** sessions that have not been seen reading (the ones that do not
read at all, and the ones that read so slowly that they could not move
the backlog in two inactivity probe cycles -- preserving all the others
that read slowly but decreased their backlog by any amount).
Another gain of such an approach is that the sessions dropped for not
reading their backlog get a log entry, which will allow their developers
to do something about it.
Cheers,
Aeliton G. Silva
On Fri, 18 Sept 2026 at 15:04, Ilya Maximets <[email protected]> wrote:
> On 9/3/26 12:07 AM, Aeliton G. Silva wrote:
> > Commit 6de8868d (reconnect: Fix broken inactivity probe if there is no
> > other reason to wake up.) enables poll_loop to run immediately on the
> > next millisecond when the probe interval has expired with no receive
> > attempted. Its reasoning notes that "in a correctly written application
> > we should not fall into this case more than once in a row".
> >
> > The problem. An ovsdb-server session whose peer stops reading falls into
> > it forever, causing the ovsdb-server process to burn CPU proportionally
> > to the number of sessions. This behaviour has been observed in
> > production, where a few ovn-octavia-provider clients raised an exception
> > but did not die, keeping their backlog frozen and degrading the
> > ovsdb-server service.
> >
> > The loop spins[1] from the moment the first interval expires -- here
> > t+8 s with the 5 s default, t+201 s with a 180 s probe -- and the cost
> > grows with the session count, because every wake-up walks every
> > connection: 10.5% of a core with the stalled session alone, 21.9%
> > alongside 50 healthy sessions, 42.3% alongside 300. Killing the stalled
> > sessions returns the pass rate and the CPU to baseline at once, which is
> > what separates the pin from load.
> >
> > A merely slow consumer arms the same pin. ovsdb-server sees only its
> > own queue shrinking, and that happens only when the kernel accepts more
> > data -- which depends on the peer acknowledging what it already has, not
> > on the peer reading each byte. The 4 kB/s row below is such a peer.
> >
> > ovsdb-server has no way to know this happened: a session with a standing
> > backlog does not have jsonrpc_session_recv() invoked
>
> Hmm. It soends like this patch is trying to fix the wrong problem.
> If we're actually never calling the recv() on a connection, that is
> likely what we should be addressing instead of inventing workarounds.
> And this should natuarally disconnect clients that do not respond to
> inactivity probes.
>
> Best regards, Ilya Maximets.
>
--
<https://www.magazineluiza.com.br/>
*Aeliton G. Silva*
Desenvolvedor Especialista - MGC SDN URP
--
_‘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
