[Serve] Reconnect long-poll clients after a controller replacement - #66511
Draft
johntaylor-cell wants to merge 3 commits into
Draft
johntaylor-cell wants to merge 3 commits into
johntaylor-cell wants to merge 3 commits into
Conversation
LongPollClient resolves its host actor once, at construction. When the ServeController is replaced rather than restarted in place, the replacement registers under the same name but with a new actor ID, so every client's next poll fails with ActorDiedError. _process_update treats any RayActorError as an intentional shutdown: it clears is_running, logs at DEBUG, and never polls again. All five production consumers (proxy, HAProxy manager, both routers, capacity queue) then stop receiving updates while serve.status() still reports RUNNING, and only restarting each consumer recovers. Re-resolve the host by name instead of retiring the client. Resolution is bounded by a deadline so an intentional serve.shutdown() still retires clients, and it runs in an executor because a blocking GCS lookup on the Ray callback thread deadlocks that thread. Fixes ray-project#63784 Signed-off-by: john.taylor <john.taylor@anyscale.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a reconnection mechanism for LongPollClient in Ray Serve, allowing it to re-resolve and reconnect to a replacement host (such as a restarted controller) when the current host dies. It includes new configuration constants, helper functions, and comprehensive unit tests to verify the reconnection behavior. The review feedback points out that the newly introduced _reconnect_task is not cancelled when stop() is called on the client, which could delay shutdown, and suggests updating the stop() method to cancel this task.
stop() cleared is_running but left a running reconnect task to notice it only after its current backoff sleep, up to 8s later. Nothing awaits the task, so shutdown was not blocked, but the task could outlive the event loop and log "Task was destroyed but it is pending". Cancel the task instead. The cancel is scheduled onto the event loop because stop() is called from long-poll listener callbacks. Signed-off-by: john.taylor <john.taylor@anyscale.com>
ray.get_actor is wrapped by Rays auto-init hook, so the reconnect loop would silently ray.init() a fresh local instance inside a process that had already called ray.shutdown(). In the serve test suites, which cycle serve.start() and serve.shutdown() in one driver process, that new instance bound the metrics export port and the following ray.init() failed with "Perhaps you called ray.init twice by accident?", breaking premerge across test_metrics, test_standalone_2, test_telemetry and others. Retire the client when Ray is gone instead, and never reach for the resolver once it is. Signed-off-by: john.taylor <john.taylor@anyscale.com>
johntaylor-cell
marked this pull request as draft
September 27, 2026 17:39
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LongPollClient resolves its host actor once, at construction. When the ServeController is replaced rather than restarted in place, the replacement registers under the same name but with a new actor ID, so every client's next poll fails with ActorDiedError. _process_update treats any RayActorError as an intentional shutdown: it clears is_running, logs at DEBUG, and never polls again. All five production consumers (proxy, HAProxy manager, both routers, capacity queue) then stop receiving updates while serve.status() still reports RUNNING, and only restarting each consumer recovers.
Re-resolve the host by name instead of retiring the client. Resolution is bounded by a deadline so an intentional serve.shutdown() still retires clients, and it runs in an executor because a blocking GCS lookup on the Ray callback thread deadlocks that thread.
Fixes #63784