ump45nose commented on code in PR #2816:
URL:
https://github.com/apache/datafusion-ballista/pull/2816#discussion_r4533266179
##########
ballista/core/src/utils.rs:
##########
@@ -452,6 +452,28 @@ where
}
}
+/// Converts a TCP keepalive interval in seconds to the idle time passed to
+/// tonic and `socket2`. A value of 0 disables keepalive.
+pub fn tcp_keepalive(seconds: u64) -> Option<Duration> {
Review Comment:
Yes, done in 4b9b936. `create_grpc_client_connection` and
`create_grpc_client_endpoint` now use `tcp_keepalive`, so
`ballista.client.grpc_tcp_keepalive_seconds = 0` disables TCP keepalive on the
client side too. I also noted the `0` behaviour on the `GrpcClientConfig` /
`GrpcServerConfig` fields and in the client config entry, and regenerated
`configs.md` with `dev/update_config_docs.sh`.
(AI-assisted reply.)
##########
ballista/scheduler/src/scheduler_process.rs:
##########
@@ -258,20 +258,35 @@ 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);
Review Comment:
Renamed it to `keepalive` in 4b9b936.
(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]
