ump45nose commented on code in PR #2816:
URL:
https://github.com/apache/datafusion-ballista/pull/2816#discussion_r4526967332
##########
ballista/scheduler/src/standalone.rs:
##########
@@ -120,11 +121,18 @@ pub async fn new_standalone_scheduler_with_builder(
info!(
"Ballista Scheduler v{BALLISTA_VERSION} (DataFusion
v{DATAFUSION_VERSION}) listening on {addr:?}"
);
+ let grpc_server_config = GrpcServerConfig::default();
+ let incoming = TcpIncoming::from(listener)
Review Comment:
Done in 64f7275: `tcp_incoming_from_listener` in `ballista_core::utils` now
owns the `with_nodelay` / `with_keepalive` chain. `create_grpc_server_incoming`
and the standalone scheduler both use it, so those settings cannot drift.
(AI-assisted reply.)
##########
ballista/scheduler/src/scheduler_process.rs:
##########
@@ -258,20 +258,41 @@ pub async fn start_grpc_service_with_listener<
let final_route =
with_version_headers(merged).into_make_service_with_connect_info::<SocketAddr>();
- // axum, unlike tonic's server, leaves Nagle on, which delays the later
- // HTTP/2 frames of each poll response behind the client's delayed ACK
- // (~40 ms on Linux).
- let listener = listener.tap_io(|tcp| {
- if let Err(e) = tcp.set_nodelay(true) {
- warn!("Failed to set TCP_NODELAY on a scheduler connection: {e}");
- }
- });
+ let tcp_keepalive =
tcp_keepalive(config.grpc_server_tcp_keepalive_seconds);
+ let listener = listener.tap_io(move |tcp| configure_connection(tcp,
tcp_keepalive));
axum::serve(listener, final_route)
.with_graceful_shutdown(async move { shutdown.recv().await })
.await
.map_err(BallistaError::from)
}
+/// Converts a keepalive setting in seconds into the idle time passed to
+/// [`configure_connection`], where 0 disables TCP keepalive.
+fn tcp_keepalive(seconds: u64) -> Option<Duration> {
Review Comment:
Done in 64f7275: `tcp_keepalive` now lives in `ballista_core::utils` (0
means disabled) and is used by `tcp_incoming_from_listener`,
`create_grpc_server_incoming`, `create_grpc_server`, and the scheduler's
accepted-connection path. `GrpcServerConfig::tcp_keepalive_seconds = 0` no
longer passes `Some(Duration::from_secs(0))`.
(AI-assisted reply.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
