Skip to content

Commit ec24890

Browse files
apollo_infra: add tcp keepalive to remote client
Set SO_KEEPALIVE on outbound sockets via a configurable idle time (tcp_keepalive_idle_time_ms). This lets the OS detect dead peers that disappear without sending FIN/RST. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1bf3f26 commit ec24890

7 files changed

Lines changed: 158 additions & 2 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/apollo_infra/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ rstest.workspace = true
3030
serde = { workspace = true, features = ["derive"] }
3131
serde_json.workspace = true
3232
starknet_api.workspace = true
33+
static_assertions.workspace = true
3334
thiserror.workspace = true
3435
time = { workspace = true, features = ["macros"] }
3536
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
@@ -38,6 +39,7 @@ tracing.workspace = true
3839
tracing-subscriber = { workspace = true, features = ["env-filter", "json", "time"] }
3940
validator.workspace = true
4041

42+
4143
[dev-dependencies]
4244
apollo_infra_utils = { workspace = true, features = ["testing"] }
4345
apollo_metrics = { workspace = true, features = ["testing"] }
@@ -47,5 +49,6 @@ metrics.workspace = true
4749
metrics-exporter-prometheus.workspace = true
4850
once_cell.workspace = true
4951
pretty_assertions.workspace = true
52+
socket2 = { workspace = true, features = ["all"] }
5053
starknet-types-core.workspace = true
5154
strum = { workspace = true, features = ["derive"] }

crates/apollo_infra/src/component_client/remote_component_client.rs

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::marker::PhantomData;
44
use std::time::Duration;
55

66
use apollo_config::dumping::{ser_param, SerializeConfig};
7+
use apollo_config::validators::create_validation_error;
78
use apollo_config::{ParamPath, ParamPrivacyInput, SerializedParam};
89
use async_trait::async_trait;
910
use bytes::Bytes;
@@ -17,11 +18,12 @@ use hyper_util::client::legacy::Client;
1718
use hyper_util::rt::TokioExecutor;
1819
use serde::de::DeserializeOwned;
1920
use serde::{Deserialize, Serialize};
21+
use static_assertions::const_assert;
2022
use tokio::sync::Mutex;
2123
use tokio::time::Instant;
2224
use tracing::field::{display, Empty};
2325
use tracing::{debug, instrument, trace, warn};
24-
use validator::Validate;
26+
use validator::{Validate, ValidationError};
2527

2628
use super::definitions::{ClientError, ClientResult};
2729
use crate::component_definitions::{
@@ -35,10 +37,19 @@ use crate::metrics::RemoteClientMetrics;
3537
use crate::requests::LabeledRequest;
3638
use crate::serde_utils::SerdeWrapper;
3739

40+
#[cfg(test)]
41+
#[path = "remote_component_client_test.rs"]
42+
mod remote_component_client_test;
43+
3844
pub const DEFAULT_RETRIES: usize = 15;
3945
pub const REQUEST_TIMEOUT_ERROR_MESSAGE: &str = "request timed out";
4046

4147
const DEFAULT_IDLE_CONNECTIONS: usize = 10;
48+
pub(crate) const TCP_IDLE_TIMEOUT_FACTOR: f64 = 1.5;
49+
// Ensure tcp connection timeout is greater than http2 connection timeout by requiring a factor
50+
// greater than 1.
51+
const_assert!(TCP_IDLE_TIMEOUT_FACTOR > 1.0);
52+
4253
// 8 MiB — bounds memory materialized from a single response as defense in depth.
4354
const DEFAULT_MAX_RESPONSE_BODY_BYTES: usize = 8 * 1024 * 1024;
4455
const DEFAULT_IDLE_TIMEOUT_MS: u64 = 30000;
@@ -54,6 +65,9 @@ const DEFAULT_REQUEST_TIMEOUT_MS: u64 = 30_000;
5465
pub struct RemoteClientConfig {
5566
pub retries: usize,
5667
pub idle_connections: usize,
68+
// Determines client connection timeouts. Used plainly for HTTP/2 connections, and with a
69+
// `TCP_IDLE_TIMEOUT_FACTOR` for TCP connections.
70+
#[validate(custom(function = "validate_tcp_exceeds_http_keepalive"))]
5771
pub idle_timeout_ms: u64,
5872
pub attempts_per_log: usize,
5973
pub initial_retry_delay_ms: u64,
@@ -81,6 +95,32 @@ impl Default for RemoteClientConfig {
8195
}
8296
}
8397

98+
/// Validates that the TCP keepalive duration (at second granularity, as the OS stores
99+
/// `TCP_KEEPIDLE` in whole seconds) is greater than or equal to the HTTP keepalive duration
100+
/// (millisecond granularity). If the configured `idle_timeout_ms * TCP_IDLE_TIMEOUT_FACTOR` is
101+
/// less than 1 second, truncation to whole seconds yields 0 s, making the TCP keepalive shorter
102+
/// than the HTTP keepalive.
103+
fn validate_tcp_exceeds_http_keepalive(idle_timeout_ms: u64) -> Result<(), ValidationError> {
104+
let http_keepalive = Duration::from_millis(idle_timeout_ms);
105+
let tcp_keepalive_raw = http_keepalive.mul_f64(TCP_IDLE_TIMEOUT_FACTOR);
106+
// TCP_KEEPIDLE is stored in whole seconds; fractional seconds are truncated by the OS.
107+
let tcp_keepalive = Duration::from_secs(tcp_keepalive_raw.as_secs());
108+
if tcp_keepalive >= http_keepalive {
109+
Ok(())
110+
} else {
111+
Err(create_validation_error(
112+
format!(
113+
"TCP keepalive ({} s) is shorter than HTTP keepalive ({idle_timeout_ms} ms): \
114+
increase idle_timeout_ms so that idle_timeout_ms * {TCP_IDLE_TIMEOUT_FACTOR} \
115+
rounds to at least {idle_timeout_ms} ms",
116+
tcp_keepalive.as_secs(),
117+
),
118+
"tcp_keepalive_shorter_than_http_keepalive",
119+
"TCP keepalive (second granularity) must be >= HTTP keepalive (ms granularity).",
120+
))
121+
}
122+
}
123+
84124
impl SerializeConfig for RemoteClientConfig {
85125
fn dump(&self) -> BTreeMap<ParamPath, SerializedParam> {
86126
BTreeMap::from_iter([
@@ -184,13 +224,19 @@ where
184224
metrics: &'static RemoteClientMetrics,
185225
) -> Self {
186226
let uri = format!("http://{url}:{port}/").parse().unwrap();
227+
let idle_timeout = Duration::from_millis(config.idle_timeout_ms);
228+
229+
// Create the tcp connector.
187230
let mut connector = HttpConnector::new();
188231
connector.set_nodelay(config.set_tcp_nodelay);
189232
connector.set_connect_timeout(Some(Duration::from_millis(config.connection_timeout_ms)));
233+
connector.set_keepalive(Some(idle_timeout.mul_f64(TCP_IDLE_TIMEOUT_FACTOR)));
234+
235+
// Create the HTTP/2 client.
190236
let client = Client::builder(TokioExecutor::new())
191237
.http2_only(true)
192238
.pool_max_idle_per_host(config.idle_connections)
193-
.pool_idle_timeout(Duration::from_millis(config.idle_timeout_ms))
239+
.pool_idle_timeout(idle_timeout)
194240
.build(connector);
195241

196242
debug!("RemoteComponentClient created with URI: {uri:?}");
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
use validator::Validate;
2+
3+
use crate::component_client::RemoteClientConfig;
4+
5+
// idle_timeout_ms = 2000 ms: tcp_raw = 3000 ms, tcp_whole_secs = 3 s = 3000 ms, which is strictly
6+
// greater than http_keepalive = 2000 ms.
7+
#[test]
8+
fn tcp_keepalive_validation_passes_when_tcp_strictly_greater_than_http_keepalive() {
9+
let config = RemoteClientConfig { idle_timeout_ms: 2000, ..Default::default() };
10+
assert!(config.validate().is_ok());
11+
}
12+
13+
// idle_timeout_ms = 1000 ms: tcp_raw = 1500 ms, tcp_whole_secs = 1 s = 1000 ms, which equals
14+
// http_keepalive = 1000 ms.
15+
#[test]
16+
fn tcp_keepalive_validation_passes_when_tcp_equals_http_keepalive() {
17+
let config = RemoteClientConfig { idle_timeout_ms: 1000, ..Default::default() };
18+
assert!(config.validate().is_ok());
19+
}
20+
21+
// idle_timeout_ms = 100 ms: tcp_raw = 150 ms, tcp_whole_secs = 0 s = 0 ms, which is less than
22+
// http_keepalive = 100 ms.
23+
#[test]
24+
fn tcp_keepalive_validation_fails_when_tcp_truncated_below_http_keepalive() {
25+
let config = RemoteClientConfig { idle_timeout_ms: 100, ..Default::default() };
26+
assert!(config.validate().is_err());
27+
}

crates/apollo_infra/src/tests/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod local_component_client_server_test;
33
mod local_request_prioritization;
44
mod remote_component_client_server_test;
55
mod server_metrics_test;
6+
mod test_utils;
67

78
use std::net::IpAddr;
89
use std::sync::Arc;

crates/apollo_infra/src/tests/remote_component_client_server_test.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use tokio::sync::{Mutex, Semaphore};
3131
use tokio::task;
3232
use tokio::time::{sleep, timeout};
3333

34+
use crate::component_client::remote_component_client::TCP_IDLE_TIMEOUT_FACTOR;
3435
use crate::component_client::{
3536
ClientError,
3637
ClientResult,
@@ -56,6 +57,7 @@ use crate::component_server::{
5657
RemoteServerConfig,
5758
};
5859
use crate::serde_utils::SerdeWrapper;
60+
use crate::tests::test_utils::client_socket_keepalive_time;
5961
use crate::tests::{
6062
available_ports_factory,
6163
dummy_remote_server_config,
@@ -734,3 +736,52 @@ async fn zombie_connection_is_evicted() {
734736
connection is still open"
735737
);
736738
}
739+
740+
/// Verifies that `TCP_KEEPIDLE` on the client's outbound socket equals
741+
/// `idle_timeout_ms * TCP_IDLE_TIMEOUT_FACTOR`, confirming the socket is armed to probe after
742+
/// exactly the expected idle period.
743+
///
744+
/// Internally, hyper's `HttpConnector::set_keepalive` calls `SockRef::set_tcp_keepalive` via
745+
/// socket2 only when it establishes a TCP connection for a request. The socket therefore only
746+
/// exists — and the option is only applied — after the first `send`. The test triggers that path,
747+
/// then reads the option back through the raw file-descriptor scan to confirm config value was
748+
/// properly applied on the socket.
749+
///
750+
/// Note: an end-to-end test that the OS closes the socket after unanswered probes would require
751+
/// packet-level manipulation (e.g. iptables DROP on loopback) and is out of scope for unit tests.
752+
#[tokio::test]
753+
async fn tcp_keepalive_idle_time_matches_config() {
754+
// 2000 * 1.5 = 3000 ms = 3 s exactly; socket2 stores TCP_KEEPIDLE in whole seconds, so the
755+
// configured duration must be a whole number of seconds or the comparison fails.
756+
const IDLE_TIMEOUT_MS: u64 = 2000;
757+
let expected_keepalive_idle =
758+
Duration::from_millis(IDLE_TIMEOUT_MS).mul_f64(TCP_IDLE_TIMEOUT_FACTOR);
759+
assert_eq!(
760+
expected_keepalive_idle.subsec_nanos(),
761+
0,
762+
"IDLE_TIMEOUT_MS * TCP_IDLE_TIMEOUT_FACTOR must be a whole number of seconds"
763+
);
764+
765+
let mut ports = available_ports_factory(unique_u16!());
766+
let a_socket = ports.get_next_local_host_socket();
767+
let b_socket = ports.get_next_local_host_socket();
768+
769+
setup_for_tests(VALID_VALUE_A, a_socket, b_socket, MAX_CONCURRENCY, None).await;
770+
771+
let client = ComponentAClient::new(
772+
RemoteClientConfig { idle_timeout_ms: IDLE_TIMEOUT_MS, ..Default::default() },
773+
&a_socket.ip().to_string(),
774+
a_socket.port(),
775+
&TEST_REMOTE_CLIENT_METRICS,
776+
);
777+
778+
// Trigger the lazy TCP connect so the socket exists and keepalive options are applied.
779+
client.a_get_value().await.expect("request should succeed");
780+
781+
let actual_keepalive_idle = client_socket_keepalive_time(a_socket)
782+
.expect("SO_KEEPALIVE should be set and TCP_KEEPIDLE should be readable");
783+
assert_eq!(
784+
actual_keepalive_idle, expected_keepalive_idle,
785+
"TCP_KEEPIDLE should equal idle_timeout_ms * TCP_IDLE_TIMEOUT_FACTOR"
786+
);
787+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
use std::net::SocketAddr;
2+
use std::os::unix::io::BorrowedFd;
3+
use std::time::Duration;
4+
5+
use socket2::SockRef;
6+
7+
/// Returns the `TCP_KEEPIDLE` duration of the outbound socket in this process that is connected
8+
/// to `server_addr`, or `None` if no such socket is found or `SO_KEEPALIVE` is not enabled.
9+
pub(crate) fn client_socket_keepalive_time(server_addr: SocketAddr) -> Option<Duration> {
10+
for fd in 0_i32..4096 {
11+
// SAFETY: We only borrow the fd transiently to read socket options; `SockRef` does
12+
// not take ownership of or close the fd. Invalid fds produce errors from `peer_addr`
13+
// and `keepalive`, which we handle gracefully via `.ok()` / `.unwrap_or`.
14+
let borrowed = unsafe { BorrowedFd::borrow_raw(fd) };
15+
let sock = SockRef::from(&borrowed);
16+
if sock
17+
.peer_addr()
18+
.ok()
19+
.and_then(|a: socket2::SockAddr| a.as_socket())
20+
.is_some_and(|a| a == server_addr)
21+
{
22+
return sock.keepalive().unwrap_or(false).then(|| sock.keepalive_time().ok()).flatten();
23+
}
24+
}
25+
None
26+
}

0 commit comments

Comments
 (0)