andygrove commented on code in PR #2816:
URL:
https://github.com/apache/datafusion-ballista/pull/2816#discussion_r4506618164
##########
ballista/scheduler/Cargo.toml:
##########
@@ -98,6 +98,7 @@ prost = { workspace = true }
rand = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true, optional = true }
+socket2 = { version = "0.6", features = ["all"] }
Review Comment:
The `python Cargo.lock up to date` check is failing because
`python/Cargo.toml` pulls in `ballista-scheduler` by path, so
`python/Cargo.lock` needs the new `socket2` entry too. Running `cd python &&
cargo update -p ballista-scheduler` and committing the lockfile should fix it.
##########
ballista/scheduler/Cargo.toml:
##########
@@ -98,6 +98,7 @@ prost = { workspace = true }
rand = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true, optional = true }
+socket2 = { version = "0.6", features = ["all"] }
Review Comment:
Small one. I think `features = ["all"]` is only needed for
`tcp_keepalive_time()` in the test. `set_tcp_keepalive` and
`TcpKeepalive::with_time` work without it. Could the main dependency drop the
feature, with `socket2 = { version = "0.6", features = ["all"] }` under
`[dev-dependencies]` instead?
##########
ballista/scheduler/src/scheduler_process.rs:
##########
@@ -348,6 +369,27 @@ mod tests {
use tokio::sync::{broadcast, oneshot};
use tower::ServiceExt;
+ /// Connections accepted by the scheduler get the same TCP keepalive as
+ /// executor and result-service connections, and none when it is disabled.
+ #[tokio::test]
+ async fn configures_nodelay_and_keepalive_on_accepted_connections() {
Review Comment:
Nit: `SockRef::tcp_keepalive_time()` isn't available on Windows in socket2
0.6, so this test won't compile there. We don't run Windows tests in CI, but a
`#[cfg(not(windows))]` would keep it portable.
--
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]
