Skip to content

fix: [branch-54] bind the gRPC listener before registering with the scheduler (backport of #2225) - #2236

Merged
andygrove merged 1 commit into
apache:branch-54from
andygrove:fix/executor-bind-before-register-branch-54
Aug 6, 2026
Merged

fix: [branch-54] bind the gRPC listener before registering with the scheduler (backport of #2225)#2236
andygrove merged 1 commit into
apache:branch-54from
andygrove:fix/executor-bind-before-register-branch-54

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Backport of #2225 to branch-54. The issue it fixes is #2224.

Rationale for this change

In push-based scheduling the executor tells the scheduler about itself before its own gRPC server is listening, and the scheduler dials that port back to verify connectivity. When the callback wins the race, the executor dies at startup.

The ordering in executor_server::startup is:

  1. tokio::spawn the tonic server. Server::serve binds inside the future, so the socket does not exist yet when spawn returns, even though the "... Grpc Server listening on ..." line is already in the log.
  2. register_executor, immediately.

On the scheduler side ExecutorManager::register_executor calls test_connectivity, a single ExecutorGrpcClient::connect(...) with no retry and no backoff. If the spawned task has not reached its bind yet, that connect gets ECONNREFUSED, registration returns an error, and executor_process treats a registration error as fatal, so the executor exits instead of joining the cluster.

The opposite direction is already tolerant: the executor retries its connection to the scheduler in a loop. Only the scheduler's callback to the executor is one-shot, which is why the executor's own startup ordering has to be right.

It is timing-dependent, so it shows up rarely and on loaded machines. Outside of tests the consequence is an otherwise healthy executor failing to join, and in a restart loop it can keep failing to join.

What changes are included in this PR?

A clean cherry-pick of 984513e, unmodified.

  • ballista/core/src/utils.rs — new create_grpc_server_incoming(addr, &GrpcServerConfig), which binds the listening socket eagerly and returns tonic's TcpIncoming. tonic ignores the server builder's tcp_nodelay / tcp_keepalive when serving from a pre-bound listener, so the helper applies the same values create_grpc_server sets and keeps the two in one place. The remaining settings still come from the builder as before, so the socket is configured exactly as it was.
  • ballista/executor/src/executor_server.rsstartup binds via that helper on the current task, before spawning, and the spawned task serves with serve_with_incoming_shutdown. Registration therefore cannot run before the port is accepting connections.

One incidental improvement falls out of binding eagerly: a port conflict now fails startup directly, with the bind error, rather than being discovered later through the spawned server task.

New tests in ballista-core:

  • test_create_grpc_server_incoming_binds_eagerly — connects to the bound address while nothing is serving on it. This is the property the fix depends on, and it fails against a lazily-bound socket.
  • test_create_grpc_server_incoming_port_in_use — binding an address twice is an error rather than a panic.

Are there any user-facing changes?

No behaviour changes for a successfully started executor and no API breakage. create_grpc_server_incoming is a new public function in ballista-core, additive alongside create_grpc_server. It must be called from within a Tokio runtime, since the listener registers with the reactor; this is documented on the function.


Verified locally on the branch-54 base: cargo fmt --all -- --check is clean, and cargo check --workspace --all-targets --locked completes with no warnings on a combined stack of the six backports being proposed together. Test execution is left to CI.

…eduler (apache#2225)

* fix(executor): bind the gRPC listener before registering with the scheduler

The executor spawned its tonic server and registered with the scheduler
immediately afterwards. Server::serve binds inside the future it returns,
so the socket did not exist yet, while the scheduler's registration
handler dials that port back via a single connect with no retry. Losing
that race gave ECONNREFUSED, failed registration, and killed the executor
at startup.

Bind the listener on the current task before spawning, and serve from it
with serve_with_incoming_shutdown, so registration cannot run before the
port accepts connections. tonic ignores the builder's tcp_nodelay and
tcp_keepalive when serving from a pre-bound listener, so the new
create_grpc_server_incoming helper applies them itself.

* refactor: tighten create_grpc_server_incoming docs and tests

* test: restore port-in-use test for create_grpc_server_incoming

(cherry picked from commit 984513e)
@andygrove
andygrove marked this pull request as ready for review August 6, 2026 12:47
@andygrove
andygrove merged commit 4925f51 into apache:branch-54 Aug 6, 2026
17 checks passed
@andygrove
andygrove deleted the fix/executor-bind-before-register-branch-54 branch August 6, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants