Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(request-response): remove Config::set_connection_keep_alive #4679

Merged
merged 11 commits into from
Oct 20, 2023
2 changes: 2 additions & 0 deletions protocols/request-response/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## 0.26.0 - unreleased

- Remove `request_response::Config::set_connection_keep_alive` in favor of `SwarmBuilder::idle_connection_timeout`.
See [PR 4679](https://github.com/libp2p/rust-libp2p/pull/4679).

## 0.25.2

Expand Down
16 changes: 4 additions & 12 deletions protocols/request-response/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use crate::handler::protocol::{RequestProtocol, ResponseProtocol};
use crate::{RequestId, EMPTY_QUEUE_SHRINK_THRESHOLD};

use futures::{channel::oneshot, future::BoxFuture, prelude::*, stream::FuturesUnordered};
use instant::Instant;
use libp2p_swarm::handler::{
ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound,
ListenUpgradeError,
Expand Down Expand Up @@ -57,9 +56,6 @@ where
inbound_protocols: SmallVec<[TCodec::Protocol; 2]>,
/// The request/response message codec.
codec: TCodec,
/// The keep-alive timeout of idle connections. A connection is considered
/// idle if there are no outbound substreams.
keep_alive_timeout: Duration,
/// The timeout for inbound and outbound substreams (i.e. request
/// and response processing).
substream_timeout: Duration,
Expand Down Expand Up @@ -92,15 +88,13 @@ where
pub(super) fn new(
inbound_protocols: SmallVec<[TCodec::Protocol; 2]>,
codec: TCodec,
keep_alive_timeout: Duration,
substream_timeout: Duration,
inbound_request_id: Arc<AtomicU64>,
) -> Self {
Self {
inbound_protocols,
codec,
keep_alive: KeepAlive::Yes,
keep_alive_timeout,
substream_timeout,
outbound: VecDeque::new(),
inbound: FuturesUnordered::new(),
Expand Down Expand Up @@ -336,13 +330,11 @@ where
self.outbound.shrink_to_fit();
}

#[allow(deprecated)]
if self.inbound.is_empty() && self.keep_alive.is_yes() {
// No new inbound or outbound requests. However, we may just have
// started the latest inbound or outbound upgrade(s), so make sure
// the keep-alive timeout is preceded by the substream timeout.
let until = Instant::now() + self.substream_timeout + self.keep_alive_timeout;
self.keep_alive = KeepAlive::Until(until);
// No new inbound or outbound requests. We already check
// there is no active streams exist in swarm connection,
// so we can set keep-alive to no directly.
self.keep_alive = KeepAlive::No;
}

Poll::Pending
Expand Down
12 changes: 0 additions & 12 deletions protocols/request-response/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,28 +284,17 @@
#[derive(Debug, Clone)]
pub struct Config {
request_timeout: Duration,
connection_keep_alive: Duration,
}

impl Default for Config {
fn default() -> Self {
Self {
connection_keep_alive: Duration::from_secs(10),
request_timeout: Duration::from_secs(10),
}
}
}

impl Config {
/// Sets the keep-alive timeout of idle connections.
#[deprecated(
note = "Set a global idle connection timeout via `SwarmBuilder::idle_connection_timeout` instead."
)]
pub fn set_connection_keep_alive(&mut self, v: Duration) -> &mut Self {
self.connection_keep_alive = v;
self
}

/// Sets the timeout for inbound and outbound requests.
pub fn set_request_timeout(&mut self, v: Duration) -> &mut Self {
self.request_timeout = v;
Expand Down Expand Up @@ -717,7 +706,6 @@
self.inbound_protocols.clone(),
self.codec.clone(),
self.config.request_timeout,
self.config.connection_keep_alive,
self.next_inbound_id.clone(),
);

Expand Down Expand Up @@ -756,11 +744,11 @@
remote_address: &Multiaddr,
_: Endpoint,
) -> Result<THandler<Self>, ConnectionDenied> {
let mut handler = Handler::new(

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Compile on x86_64-pc-windows-msvc

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Check rustdoc intra-doc links

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy (nightly-2023-09-10)

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / examples

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Compile with MSRV

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-autonat

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-perf

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-rendezvous

this function takes 4 arguments but 5 arguments were supplied

Check failure on line 747 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-request-response

this function takes 4 arguments but 5 arguments were supplied
self.inbound_protocols.clone(),
self.codec.clone(),
self.config.request_timeout,
self.config.connection_keep_alive,

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Compile on x86_64-pc-windows-msvc

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Check rustdoc intra-doc links

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy (nightly-2023-09-10)

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / examples

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Compile with MSRV

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-autonat

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-perf

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-rendezvous

no field `connection_keep_alive` on type `Config`

Check failure on line 751 in protocols/request-response/src/lib.rs

View workflow job for this annotation

GitHub Actions / Test libp2p-request-response

no field `connection_keep_alive` on type `Config`
self.next_inbound_id.clone(),
);

Expand Down
Loading