ump45nose opened a new pull request, #2816:
URL: https://github.com/apache/datafusion-ballista/pull/2816
# Which issue does this PR close?
Closes #2808.
# Rationale for this change
The scheduler serves its gRPC listener with `axum::serve`, which sets no TCP
keepalive on accepted connections. A scheduler connection whose peer disappears
without closing it (a killed executor host, a dropped NAT or load-balancer
entry) is therefore never reaped by OS keepalive probes, while executor and
result-service listeners get a 3600 s keepalive from
`create_grpc_server_incoming`.
# What changes are included in this PR?
- `start_grpc_service_with_listener`: the existing `tap_io` hook now goes
through a small `configure_connection` helper that sets `TCP_NODELAY`
(unchanged) and TCP keepalive via `socket2::SockRef::set_tcp_keepalive`.
Failures are logged as warnings, like the `TCP_NODELAY` case.
- New scheduler option `grpc_server_tcp_keepalive_seconds`
(`--grpc-server-tcp-keepalive-seconds`, default 3600 to match
`GrpcServerConfig`, `0` disables) with a
`SchedulerConfig::with_grpc_server_tcp_keepalive_seconds` builder.
- Standalone scheduler: its `TcpIncoming` now also gets `with_keepalive`
from the default `GrpcServerConfig`, since tonic ignores the builder's
`tcp_keepalive` on a pre-bound listener.
- `socket2` (already in the lockfile at 0.6.5 through tokio/hyper) becomes a
direct dependency of `ballista-scheduler`.
- Unit test `configures_nodelay_and_keepalive_on_accepted_connections`
checks `TCP_NODELAY`, `SO_KEEPALIVE` and the keepalive idle time on an accepted
socket, and that `0` leaves keepalive off.
Not covered: the HTTP/2 keepalive ping interval/timeout that
`create_grpc_server` sets for tonic servers. `axum::serve` does not expose
those settings, so matching them would mean serving with hyper directly; I left
that out to keep this change small.
# Are there any user-facing changes?
One new scheduler CLI flag / `SchedulerConfig` field,
`grpc_server_tcp_keepalive_seconds` (default 3600). Scheduler connections now
get TCP keepalive by default. No API removals.
# Testing
- `cargo test -p ballista-scheduler --lib` (611 passed, 2 ignored)
- `cargo clippy -p ballista-scheduler --all-targets -- -D warnings`
- `cargo check -p ballista-scheduler --all-features --bins`
- `cargo fmt --all`
---
This PR was made with AI assistance. I reviewed the diff and ran the tests
above.
--
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]
