Release tracker actors between tests - #182
Merged
Merged
Conversation
The new actor test failed on main with an empty dataframe, having passed on the PR branch and locally. It is not a bug in the actor id fix. Both tracker actors pin 0.1 of the head node's `node:__internal_head__` budget, which totals 1.0, so each RayTaskTracker costs 0.2 and a cluster admits exactly five. Measured: the eleventh pinned actor cannot be scheduled at all. Tests only stopped the dashboard, never the detached actors, so the budget accumulated across the module. Four trackers held 0.8; adding the actor test made it five and 1.0 — exactly the cap, with no headroom. Whichever tracker lost the race got an unschedulable callback actor, which never reported back, so the dataframe stayed empty. Sitting on the boundary is why it passed in one environment and failed in another. Build trackers through a fixture that calls exit() afterwards. Verified that exit() returns the pin: seven trackers in sequence each take 0.2 and release back to 1.0. The five-tracker ceiling itself is untouched here and is a real limit on concurrent trackers per cluster. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Contributor
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.
Fixes the CI failure on main after #181. Not a bug in the actor id fix — that change is correct.
Root cause
Both tracker actors pin
node:__internal_head__: 0.1, and that resource totals 1.0 on the head node. EachRayTaskTrackercreates two of them, so it costs 0.2 and a cluster admits exactly five. Measured:The tests only called
dashboard.stop(), never releasing the detached actors, so the budget accumulated across the module. Four trackers held 0.8; the new actor test made it five and exactly 1.0 — the cap, with zero headroom. Whichever tracker lost the race got an unschedulable callback actor, which never reported back, soget_df()stayed empty for the full 120s wait.Sitting precisely on the boundary is why the same commit passed on the PR branch and locally, then failed on main.
Fix
Build trackers through a fixture that calls
exit()in teardown. Verified thatexit()returns the pin — seven trackers in sequence, each taking 0.2 and releasing back to 1.0: