You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, whether or not a connection has active streams does not affect its keep-alive. However, for most protocols, that is the default. The only exception that we know of here is the ping protocol.
As a first step, we want to incorporate "counting" of active streams in the keep-alive algorithm of a connection. This code lives here:
let effective_keep_alive = max(requested_keep_alive,*idle_timeout);
// Important: We store the _original_ `Instant` given by the `ConnectionHandler` in the `Later` instance to ensure we can compare it in the above branch.
// This is quite subtle but will hopefully become simpler soon once `KeepAlive::Until` is fully deprecated. See <https://github.com/libp2p/rust-libp2p/issues/3844>/
To count the number of active streams on a connection, we embed an Arc<()> in each stream. This is possible in a backwards-compatible way because Stream is instantiated within this module.
At the start of the connection, we construct an Arc<()> as a stream counter and store it in Connection.
Every time we make a new stream, be in inbound or outbound, we clone this Arc and store it in the Stream
At any point, we can check how many streams there are by calling Arc::strong_count.
If this count reaches 1, we know that there are no active streams on this connection. This should be a pre-condition to asking the ConnectionHandler via ConnectionHandler::connection_keep_alive whether or not the connection is still needed.
Overall, this will allow protocols to not having to do any tracking on their own in regards to whether to keep the connection alive because there are still streams running on it.
As a second step, we need to introduce an "opt-out" functionality for this behaviour:
implStream{pubfnno_keep_alive(&mutself);}
Internally, this needs to downgrade the Arc<()> to a Weak<()> which automatically reduces the strong_count and thus "ignores" this stream. The ping protocol needs to call this function on every Stream received to not keep the connection alive if the only remaining streams are ping-streams.
Within the same PR, we should remove any book-keeping from protocols around active streams to make sure that this mechanism actually works.
The text was updated successfully, but these errors were encountered:
Currently, whether or not a connection has active streams does not affect its keep-alive. However, for most protocols, that is the default. The only exception that we know of here is the ping protocol.
As a first step, we want to incorporate "counting" of active streams in the keep-alive algorithm of a connection. This code lives here:
rust-libp2p/swarm/src/connection.rs
Lines 348 to 386 in c52a2fc
To count the number of active streams on a connection, we embed an
Arc<()>
in each stream. This is possible in a backwards-compatible way becauseStream
is instantiated within this module.Arc<()>
as a stream counter and store it inConnection
.clone
thisArc
and store it in theStream
Arc::strong_count
.If this count reaches
1
, we know that there are no active streams on this connection. This should be a pre-condition to asking theConnectionHandler
viaConnectionHandler::connection_keep_alive
whether or not the connection is still needed.Overall, this will allow protocols to not having to do any tracking on their own in regards to whether to keep the connection alive because there are still streams running on it.
As a second step, we need to introduce an "opt-out" functionality for this behaviour:
Internally, this needs to downgrade the
Arc<()>
to aWeak<()>
which automatically reduces thestrong_count
and thus "ignores" this stream. The ping protocol needs to call this function on everyStream
received to not keep the connection alive if the only remaining streams are ping-streams.Within the same PR, we should remove any book-keeping from protocols around active streams to make sure that this mechanism actually works.
The text was updated successfully, but these errors were encountered: