Skip to content

Commit 527a8f7

Browse files
apollo_infra: add tcp keepalive to remote server accepted sockets
Mirror the client-side SO_KEEPALIVE behaviour on the server: set TCP keepalive probes on each accepted socket via a configurable idle time (tcp_keepalive_idle_time_ms). This lets the OS detect dead clients that disappear without sending FIN/RST. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 56bc1b5 commit 527a8f7

6 files changed

Lines changed: 105 additions & 10 deletions

File tree

crates/apollo_infra/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ rand.workspace = true
2929
rstest.workspace = true
3030
serde = { workspace = true, features = ["derive"] }
3131
serde_json.workspace = true
32+
socket2 = { workspace = true, features = ["all"] }
3233
starknet_api.workspace = true
3334
static_assertions.workspace = true
3435
thiserror.workspace = true
@@ -49,6 +50,5 @@ metrics.workspace = true
4950
metrics-exporter-prometheus.workspace = true
5051
once_cell.workspace = true
5152
pretty_assertions.workspace = true
52-
socket2 = { workspace = true, features = ["all"] }
5353
starknet-types-core.workspace = true
5454
strum = { workspace = true, features = ["derive"] }

crates/apollo_infra/src/component_client/definitions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use serde::de::DeserializeOwned;
33
use serde::Serialize;
44
use thiserror::Error;
55

6-
use super::{LocalComponentClient, RemoteComponentClient};
6+
use crate::component_client::{LocalComponentClient, RemoteComponentClient};
77
use crate::component_definitions::ServerError;
88

99
#[derive(Clone, Debug, Error, PartialEq, Eq)]

crates/apollo_infra/src/component_client/remote_component_client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use tracing::field::{display, Empty};
2424
use tracing::{debug, instrument, trace, warn};
2525
use validator::{Validate, ValidationError};
2626

27-
use super::definitions::{ClientError, ClientResult};
27+
use crate::component_client::{ClientError, ClientResult};
2828
use crate::component_definitions::{
2929
ComponentClient,
3030
RequestId,

crates/apollo_infra/src/component_server/remote_component_server.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ use hyper_util::rt::{TokioExecutor, TokioIo, TokioTimer};
2020
use hyper_util::server::conn::auto::Builder as ServerBuilder;
2121
use serde::de::DeserializeOwned;
2222
use serde::{Deserialize, Serialize};
23+
use socket2::{SockRef, TcpKeepalive};
2324
use tokio::net::TcpListener;
2425
use tokio::sync::{OwnedSemaphorePermit, Semaphore};
2526
use tracing::{debug, error, instrument, trace, warn};
2627
use validator::Validate;
2728

29+
use crate::component_client::remote_component_client::validate_keepalive_timeout_ms;
2830
use crate::component_client::{ClientError, LocalComponentClient};
2931
use crate::component_definitions::{
3032
ComponentClient,
@@ -33,6 +35,7 @@ use crate::component_definitions::{
3335
APPLICATION_OCTET_STREAM,
3436
BUSY_PREVIOUS_REQUESTS_MSG,
3537
REQUEST_ID_HEADER,
38+
TCP_KEEPALIVE_FACTOR,
3639
};
3740
use crate::component_server::ComponentServerStarter;
3841
use crate::metrics::RemoteServerMetrics;
@@ -46,6 +49,9 @@ const DEFAULT_MAX_CONCURRENCY: usize = 128;
4649
const DEFAULT_MAX_REQUEST_BODY_BYTES: usize = 8 * 1024 * 1024;
4750
const DEFAULT_KEEPALIVE_INTERVAL_MS: u64 = 30_000;
4851
const DEFAULT_KEEPALIVE_TIMEOUT_MS: u64 = 10_000;
52+
// Number of unanswered TCP keepalive probes before the OS declares the connection dead.
53+
// 3 probes × keepalive_interval gives a ~90 s probe window at the default interval.
54+
const TCP_KEEPALIVE_RETRIES: u32 = 3;
4955

5056
macro_rules! serve_connection {
5157
(
@@ -80,6 +86,7 @@ pub struct RemoteServerConfig {
8086
pub max_concurrency: usize,
8187
pub max_request_body_bytes: usize,
8288
pub keepalive_interval_ms: u64,
89+
#[validate(custom(function = "validate_keepalive_timeout_ms"))]
8390
pub keepalive_timeout_ms: u64,
8491
}
8592

@@ -393,6 +400,10 @@ where
393400
panic!("Failed to bind remote component server socket {:#?}: {e}", bind_socket)
394401
});
395402

403+
let max_streams = self.config.max_streams_per_connection;
404+
let keepalive_interval = Duration::from_millis(self.config.keepalive_interval_ms);
405+
let keepalive_timeout = Duration::from_millis(self.config.keepalive_timeout_ms);
406+
396407
loop {
397408
let (stream, peer_addr) = match listener.accept().await {
398409
Ok(conn) => conn,
@@ -407,10 +418,15 @@ where
407418
warn!("Failed to set TCP_NODELAY: {e}");
408419
}
409420

421+
let tcp_keepalive = TcpKeepalive::new()
422+
.with_time(keepalive_timeout.mul_f64(TCP_KEEPALIVE_FACTOR))
423+
.with_interval(keepalive_interval)
424+
.with_retries(TCP_KEEPALIVE_RETRIES);
425+
if let Err(e) = SockRef::from(&stream).set_tcp_keepalive(&tcp_keepalive) {
426+
error!("Failed to set TCP keepalive: {e}");
427+
}
428+
410429
let io = TokioIo::new(stream);
411-
let max_streams = self.config.max_streams_per_connection;
412-
let keepalive_interval = Duration::from_millis(self.config.keepalive_interval_ms);
413-
let keepalive_timeout = Duration::from_millis(self.config.keepalive_timeout_ms);
414430

415431
tokio::spawn(per_connection_service(
416432
io,

crates/apollo_infra/src/tests/remote_component_client_server_test.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -651,13 +651,13 @@ async fn retry_request() {
651651
async fn tcp_keepalive_idle_time_matches_config() {
652652
// 2000 * 1.5 = 3000 ms = 3 s exactly; socket2 stores TCP_KEEPIDLE in whole seconds, so the
653653
// configured duration must be a whole number of seconds or the comparison fails.
654-
const IDLE_TIMEOUT_MS: u64 = 2000;
654+
const KEEPALIVE_TIMEOUT_MS: u64 = 2000;
655655
let expected_keepalive_idle =
656-
Duration::from_millis(IDLE_TIMEOUT_MS).mul_f64(TCP_KEEPALIVE_FACTOR);
656+
Duration::from_millis(KEEPALIVE_TIMEOUT_MS).mul_f64(TCP_KEEPALIVE_FACTOR);
657657
assert_eq!(
658658
expected_keepalive_idle.subsec_nanos(),
659659
0,
660-
"IDLE_TIMEOUT_MS * TCP_KEEPALIVE_FACTOR must be a whole number of seconds"
660+
"KEEPALIVE_TIMEOUT_MS * TCP_KEEPALIVE_FACTOR must be a whole number of seconds"
661661
);
662662

663663
let mut ports = available_ports_factory(unique_u16!());
@@ -667,7 +667,7 @@ async fn tcp_keepalive_idle_time_matches_config() {
667667
setup_for_tests(VALID_VALUE_A, a_socket, b_socket, MAX_CONCURRENCY, None).await;
668668

669669
let client = ComponentAClient::new(
670-
RemoteClientConfig { keepalive_timeout_ms: IDLE_TIMEOUT_MS, ..Default::default() },
670+
RemoteClientConfig { keepalive_timeout_ms: KEEPALIVE_TIMEOUT_MS, ..Default::default() },
671671
&a_socket.ip().to_string(),
672672
a_socket.port(),
673673
&TEST_REMOTE_CLIENT_METRICS,

crates/apollo_infra/src/tests/remote_server_connection_eviction_test.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
12
use std::time::Duration;
23

34
use apollo_proc_macros::unique_u16;
5+
use rstest::rstest;
6+
use socket2::{SockRef, TcpKeepalive};
47
use tokio::io::AsyncReadExt;
8+
use tokio::net::{TcpListener, TcpStream};
59
use tokio::sync::mpsc::channel;
610
use tokio::task;
711
use tokio::time::{sleep, timeout};
@@ -67,3 +71,78 @@ async fn zombie_connection_is_evicted_via_http_keepalive() {
6771
connection is still open"
6872
);
6973
}
74+
75+
/// Verifies that `SO_KEEPALIVE` on a server-accepted socket reflects `idle_time_ms`.
76+
///
77+
/// The test accepts the connection itself so it owns the `TcpStream` and can inspect socket
78+
/// options via `SockRef::from` without any unsafe FD scanning.
79+
#[rstest]
80+
#[tokio::test]
81+
async fn server_tcp_keepalive_socket_option_matches_config() {
82+
// Linux TCP_KEEPIDLE has 1-second granularity; values below 1000ms round to 0 and fail.
83+
const ARBITRARY_IDLE_TIMEOUT_MS: u64 = 1000;
84+
85+
let listener =
86+
TcpListener::bind(SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 0)).await.unwrap();
87+
let server_addr = listener.local_addr().unwrap();
88+
89+
let _client_stream = TcpStream::connect(server_addr).await.unwrap();
90+
let (accepted_stream, _) = listener.accept().await.unwrap();
91+
92+
// Mirror the keepalive logic in RemoteComponentServer::start().
93+
let keepalive = TcpKeepalive::new().with_time(Duration::from_millis(ARBITRARY_IDLE_TIMEOUT_MS));
94+
SockRef::from(&accepted_stream).set_tcp_keepalive(&keepalive).unwrap();
95+
96+
assert!(
97+
SockRef::from(&accepted_stream).keepalive().unwrap(),
98+
"SO_KEEPALIVE on the accepted socket should reflect idle_time_ms"
99+
);
100+
}
101+
102+
/// Verifies that a `RemoteComponentServer` configured with TCP keepalive correctly evicts
103+
/// zombie connections.
104+
///
105+
/// On loopback the OS always acknowledges TCP keepalive probes, so the connection is evicted
106+
/// via the HTTP/2 PING mechanism rather than TCP keepalive itself. The test verifies that
107+
/// enabling TCP keepalive does not interfere with connection eviction.
108+
#[tokio::test]
109+
async fn server_with_tcp_keepalive_evicts_zombie_connection() {
110+
const KEEPALIVE_INTERVAL_MS: u64 = 100;
111+
const KEEPALIVE_TIMEOUT_MS: u64 = 100;
112+
const MARGIN_MS: u64 = 500;
113+
114+
let socket = available_ports_factory(unique_u16!()).get_next_local_host_socket();
115+
116+
let (tx, _rx) = channel::<RequestWrapper<ComponentARequest, ComponentAResponse>>(32);
117+
let local_client = LocalComponentClient::<ComponentARequest, ComponentAResponse>::new(
118+
tx,
119+
&TEST_LOCAL_CLIENT_METRICS,
120+
);
121+
let config = RemoteServerConfig {
122+
keepalive_interval_ms: KEEPALIVE_INTERVAL_MS,
123+
keepalive_timeout_ms: KEEPALIVE_TIMEOUT_MS,
124+
..dummy_remote_server_config(socket.ip(), MAX_CONCURRENCY)
125+
};
126+
let mut server = RemoteComponentServer::new(
127+
local_client,
128+
config,
129+
socket.port(),
130+
&TEST_REMOTE_SERVER_METRICS,
131+
);
132+
task::spawn(async move { server.start().await });
133+
task::yield_now().await;
134+
135+
let mut zombie = connect_zombie(socket).await;
136+
137+
let mut buf = Vec::new();
138+
let read_result = timeout(
139+
Duration::from_millis(KEEPALIVE_INTERVAL_MS + KEEPALIVE_TIMEOUT_MS + MARGIN_MS),
140+
zombie.read_to_end(&mut buf),
141+
)
142+
.await;
143+
assert!(
144+
read_result.is_ok(),
145+
"Server should have closed the zombie connection after keepalive timeout, but the \
146+
connection is still open"
147+
);
148+
}

0 commit comments

Comments
 (0)