fix(RayAppMaster): refactor executor state in RayAppMaster. - #487
Open
my-vegetable-has-exploded wants to merge 3 commits into
Open
fix(RayAppMaster): refactor executor state in RayAppMaster.#487my-vegetable-has-exploded wants to merge 3 commits into
my-vegetable-has-exploded wants to merge 3 commits into
Conversation
added 3 commits
July 23, 2026 08:32
…estart handling fix(RayAppMaster): enhance executor management with actor slots and restart handling Signed-off-by: epsilonwang <epsilonwang@didiglobal.com> Reviewed By: fanshilun CR Link: https://kunpeng.xiaojukeji.com/view/revision/6286929
…from Ray actorId [DIDIRAYDP-9]Refactor executor tracking to separate Spark executorId from Ray actorId Replace the single executorIdToHandler map with executorIdToActorId + actorIdToHandle, reflecting that Ray actors keep their original actorId across restarts while Spark assigns a new executorId each generation. This fixes RequestAddPendingRestartedExecutor to use actorId (not executorId) for named actor lookup, and fixes a use-after-delete bug in kill() where executorIdToHandler was removed before being read. Signed-off-by: epsilonwang <epsilonwang@didiglobal.com> Reviewed By: mazhengxuan CR Link: https://kunpeng.xiaojukeji.com/view/revision/6376221
[DIDIRAYDP-10] Adds executor reconciliation after removal Ensures that after an executor is removed from the startup registry, a reconciliation is triggered to maintain consistency between the expected and actual executor state. This prevents potential resource leaks or state mismatches during executor lifecycle management. Signed-off-by: epsilonwang <epsilonwang@didiglobal.com> Reviewed By: fanshilun,epsilonwang CR Link: https://kunpeng.xiaojukeji.com/view/revision/6392741
Contributor
Author
|
Hi @pang-wu @carsonwang , could you please review this PR? This fix addresses a real issue we encountered in production. |
Contributor
Author
|
@claude review |
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.
What does this PR do?
The old code counted Spark executor generations instead of logical Ray actors. A restarted
executor was recorded in both
executorsandrestartedExecutors, while stale restart mappingssurvived executor removal. This allowed
registeredExecutorsto report a deficit while therestart-based limit check simultaneously reported that the application was full.
RequestExecutorswould then acknowledge the target without creating any actor. Tracking slotsby the stable Ray
actorIdremoves this contradiction: a restart changes the Spark executorgeneration, not the number of logical actors.
Two lifecycle details are essential to make that model correct. First, the actor-slot registry must
retain an
ActorHandleacross executor disconnects; otherwise Ray's reference can be released andthe non-detached actor terminated while RayDP still counts its slot. Second, RayAppMaster must
reconcile after
KillExecutors: Spark treats the requested total as persistent and expects thecluster manager, as YARN does, to replace missing executors without another identical request from
the driver.
Why is this change needed?
Problem
RayDP previously conflated two different identities:
actorIdacross restarts.executorIdfor every restarted generation.The previous implementation used several generation-based states to approximate the number of
executor actors:
registeredExecutorsto calculate how many executors were missing.executors.size + restartedExecutors.sizeto enforce the executor limit.restartedExecutorsto retain the mapping from a new executor ID to its original actor ID.These states have different lifecycles and can disagree. In particular, a restarted executor is
present in both
executorsandrestartedExecutors, so the same Ray actor is counted twice.Moreover, entries in
restartedExecutorsare not removed when the corresponding executorgeneration disconnects, allowing historical restarts to remain in executor accounting.
Failure Scenario
The incorrect count follows directly from three asymmetric code paths:
addPendingRegisterExecutor(newExecutorId, ...), which inserts the new generation intoappInfo.executors. It then also writesrestartedExecutors(newExecutorId) = originalActorId.newExecutorIdis unique. A later restart therefore adds another map entry instead ofreplacing the entry for the previous generation, even though both values point to the same
original Ray actor.
ApplicationInfo.kill()removes the current generation fromappInfo.executors. It cannot remove the matchingrestartedExecutorsentry because that mapis owned separately by
RayAppMaster, and neither the disconnect path norKillExecutorsperforms such cleanup.
The capacity check nevertheless uses:
This is wrong in two different phases. While a restarted generation is active, its executor ID is
present in both maps and is counted twice. After it disconnects, it is removed from
executorsbutremains in
restartedExecutors, so it becomes a permanently counted historical generation.For one logical actor with original
actorId = 7, the state evolves as follows:appInfo.executorsrestartedExecutors{7}{}{1001}{1001 -> 7}{}{1001 -> 7}{1002}{1001 -> 7, 1002 -> 7}Thus, each restart adds another capacity unit to
restartedExecutors, although the number of Rayactor slots never changes. Disconnecting the generation does not undo that increase.
The restart-registration path does not call
requestNewExecutor(), so it bypasses the maximumcheck that later consumes this inflated count. Each disconnect also decrements
registeredExecutors, makingremainingUnRegisteredExecutors > 0true and admitting the nextrestart generation. The history can therefore grow to the configured maximum and beyond.
The key contradiction is:
Both cannot be valid measures of Ray executor capacity.
Solution
Track logical Ray actor slots independently from Spark executor generations.
An actor slot is keyed by the original
actorId:executorId, but does not add another slot.same actor.
KillExecutors, RayDP removes the actor slot and requests actor shutdown.desiredExecutors.These rules describe two stages that may occur in the same failure path. The AppMaster's initial
RPC disconnect handling keeps the slot because Ray may recover the actor. If Spark subsequently
decides to terminate that executor and sends
KillExecutors, RayDP then removes the slot andstops the actor. Therefore slot removal is tied to the
KillExecutorscommand, not only to anexplicit scale-down reason.
Executor reconciliation then becomes:
This compares two values with matching semantics: the number of logical actors requested by Spark
and the number of logical Ray actor slots currently owned by the application.
The implementation keeps an
actorId -> ActorHandleregistry as the actor-slot state and maps eachSpark executor generation back to its stable actor ID. The restarted-executor mapping required by
recoverable RDD lookup is derived from this state, but is no longer used for capacity accounting.
The actor-slot registry cannot continue to use the previous
executorIdToHandlermap. When anexecutor disconnects,
onDisconnectedcallsApplicationInfo.kill(..., shutdownActor = false)toremove that Spark executor generation while allowing Ray to restart its actor. However,
kill()unconditionally executes
executorIdToHandler -= executorId. Removing the executor ID also removesthe map entry that retains the corresponding actor handle. If this is the last retained handle, the
handle becomes unreachable and the actor can be terminated, which violates the expected restart
semantics of
shutdownActor = false.The handle is not only a lookup convenience: it keeps Ray's reference to a non-detached actor
alive. RayDP creates a named actor with
setName, but does not give itActorLifetime.DETACHED.In Ray 2.1,
NativeActorHandleReference.finalizeReferent()callsnativeRemoveActorHandleReference()after aJava handle becomes unreachable. Losing the last retained handle can therefore reduce the
distributed reference count to zero and terminate the actor.
setMaxRestarts(-1)allows recoveryfrom actor process failures; it does not make the actor independent of its owning reference.
The sharp failure sequence is:
RayDP is then left with a ghost slot:
actorSlotssays actor 23 exists, while the actor and itshandle are gone. Because reconciliation sees the slot as present, it creates no replacement. A
named actor is not sufficient protection here; naming makes lookup possible only while the actor
is still alive.
For this reason,
f61f80e5e6e0daa125172573ecf4c8262aea2af1replacesexecutorIdToHandlerwith two maps:The first map follows the lifetime of a Spark executor generation. The second is both the actor-slot
registry and the strong-reference holder for the Ray actor. Removing a disconnected executor ID
now deletes only the generation mapping. The stable actor handle remains until the actor slot itself
is deliberately removed, so Ray can restart the actor and register a new Spark
executorId.Reconcile Immediately after
KillExecutorsSpark's total-executor request is a persistent desired-state contract, not a one-shot request to
create a delta.
CoarseGrainedSchedulerBackendexplicitly assumes that the cluster manager willeventually fulfill an acknowledged total request when resources become available. Consequently,
the Spark driver does not periodically resend the same target merely because an executor
disappears; the resource-management side is responsible for retaining the target and reconciling
its actual resources against it.
YARN demonstrates this contract directly.
YarnAllocatorstores the driver's total intargetNumExecutorsPerResourceProfileId. On every allocator heartbeat, it computes:When a container exits, the running count decreases and the next heartbeat requests a replacement,
without requiring the Spark driver to resend its unchanged target.
Before this commit, RayDP retained
desiredExecutors, but reconciled only when it receivedRequestExecutors. Consider a target of 1000 and Spark callingkillAndReplaceExecutor(17). Sparksends
KillExecutorswithadjustTargetNumExecutors = false, so its desired target correctly staysat 1000. RayAppMaster removes actor slot 17 and acknowledges the kill, leaving 999 slots. No new
RequestExecutors(1000)is required by Spark's contract, and RayAppMaster had neither a periodicallocator loop nor a reconciliation call on this state transition. The application could therefore
remain at 999 indefinitely; repeated replacement kills could accumulate the deficit.
This commit invokes
reconcileExecutors()immediately after processingKillExecutors:desiredExecutorsalready matchesthe reduced slot count and no replacement is created.
removed slot immediately appears as a deficit and RayDP creates a replacement.
RequestExecutorsandKillExecutorsare the two events that change RayAppMaster's target or actorslot count. Reconciling on both events gives RayDP the same desired-state behavior that Spark expects
from resource managers such as YARN, without requiring a periodic target synchronization from the
driver.
Related Issue
Fixes #
Type of Change
How was this tested?
This fix has been tested in our production environment and works well with dynamic execution enabled and 1,000 executors. Before this fix, large-scale jobs would occasionally fail.
Checklist