fix(miles-service): use force=False in _cancel_inflight — force=True is a no-op for actor tasks#39
Open
JunzheJoe wants to merge 1 commit into
Open
Conversation
…is a no-op for actor tasks
Ray only allows force=False when cancelling actor tasks; force=True
raises ValueError before cancelling anything ('Only force=False is
allowed for an Actor Task', Ray 2.55 docs). Every ref in inflight_refs
is an actor-method ObjectRef, so the R06-F1 timeout fan-out never
cancelled a single task — each ValueError was swallowed by the per-ref
try/except and logged as a warning.
With force=False the fan-out does what it actually can: queued tasks
(finalize / set_weight_version / try_put behind a wedged transport) are
cancelled and never start. A task already executing on the sync
cache_owner / rollout_manager actors still runs to completion — Ray
only sets a cooperative is_canceled() flag that miles does not poll —
so the R06-F1 comment is rewritten to describe the real, narrower
guarantee instead of claiming interruption.
Cooperative interruption of the executing run_sync_session (polling
is_canceled() in the bucket loop, or moving the service chain to an
async actor) is a follow-up design decision.
Test: tests/test_miles_service_cancel_inflight.py pins that every ref
reaches ray.cancel with force=False (the stub mirrors Ray's contract by
raising on force=True, which is exactly the pre-fix silent-no-op).
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
1 participant
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
Review finding F56-CANCEL-SYNC (F1-F12 review, confirmed against the Ray docs on 2026-07-06 — see
docs/m11-joe-f1-f12-review.md§5 F5+F6).Ray's
ray.cancelcontract for actor tasks: "Onlyforce=Falseis allowed for an Actor Task. Otherwise, it raisesValueError."Every ref in
inflight_refsis an actor-method ObjectRef (run_sync_session/finalize_weight_update/set_weight_version/get_free_port/try_put), so the R06-F1 timeout fan-outray.cancel(ref, force=True)raisedValueErroron every single ref — swallowed by the per-reftry/except, logged as a warning. Net effect: the whole cancellation mechanism was a silent no-op; not even queued tasks were ever cancelled.Fix (minimal)
force=True→force=False: queued follow-up RPCs behind a wedged transport are now genuinely cancelled and never start.is_canceled()flag that miles does not poll) — it runs to completion and keeps_cache_lockuntil then. The old comment claimed interruption that Ray never provided.Not in scope
Cooperative interruption of an executing
run_sync_session(pollingis_canceled()inside the bucket loop, or making the service chain async-actor-based) is a real design change — deferred for team discussion.Adjacent, not conflicting: #23 adds pause/continue refs into
inflight_refs; this PR makes the cancel they'd receive actually work. The two compose — #23's refs are exactly the queued-task kind this fix can cancel.Test
tests/test_miles_service_cancel_inflight.py— the ray stub mirrors Ray's contract (raises onforce=True), pinning that every ref reachesray.cancelwithforce=Falseand the list drains. Suite: 60 passed, 3 skipped.