Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions scripts/ci/owned_pool_rescue.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@
cancelled and not re-run (so the newer run starts), and a refused one is left
as it is; the newer run builds main's newer HEAD.

A job stuck with no runner is not rescued while another job of its run is
running on a persistent runner, whatever the workflow: the rescue cancels the
whole run, which would move that job to Blacksmith too (#16463). The stuck
job waits until that job ends, and if the watch ends first, it stays queued
for the mini that frees up. A refused job, or one held in setup at the
watch's end, is still acted on as described above, running siblings or not.

A job's wait is measured from the later of its `created_at` and the first
time the watcher saw it queued, so a job record created before its `needs`
were met can never count as already past the budget.
Expand Down Expand Up @@ -547,8 +554,14 @@ def assess(jobs: Sequence[Mapping[str, Any]], *, now: dt.datetime, budget_second
budgets = {id(job): job_budget(job, budget_seconds, deadline=deadline, floor_seconds=floor_seconds,
first_seen=seen.get(job.get("id"))) for job in waiting}
stuck = [job for job in waiting if queued_seconds(job, now, seen.get(job.get("id"))) >= budgets[id(job)]]
if stuck:
names = ", ".join(sorted(str(job.get("name") or job.get("id")) for job in stuck))
names = ", ".join(sorted(str(job.get("name") or job.get("id")) for job in stuck))
# Rescuing cancels the whole run, so a job already running on a persistent
# runner would die with the stuck one and move to Blacksmith too (#16463:
# a cmux-next swift test three minutes into its mini). The stuck job waits
# until the runner's job ends, even past the watch's end, when a mini that
# frees up takes it. A held or refused job is still judged below.
on_mini = [job for job in jobs if job_pool(job) and job.get("status") == "in_progress" and not in_setup(job)]
if stuck and not on_mini:
return Look("rescue", f"{names} queued on {job_pool(stuck[0])} for at least "
f"{min(budgets[id(job)] for job in stuck)}s with no runner")
settling = [job for job in jobs if job_pool(job) and in_setup(job)]
Expand All @@ -566,6 +579,9 @@ def assess(jobs: Sequence[Mapping[str, Any]], *, now: dt.datetime, budget_second
if turned_away:
names = ", ".join(sorted(str(job.get("name") or job.get("id")) for job in turned_away))
return Look("refused", f"{names} refused by {job_pool(turned_away[0])} at job start or Xcode selection")
if stuck:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A stuck job held back by a mini-running sibling has no recovery path once the watch's deadline passes. The watch loop returns "stop: watch limit reached" with nothing cancelled, and later sweepers skip this run as soon as its marker passes SWEEP_MAX_AGE_SECONDS (150 min; sweep() drops marked runs whose created is older than oldest). If the pool stays busy past that point the shard waits indefinitely and is never rescued or re-run. The narrower new case this hold also enables: if the run reaches "completed" while the held job is still queued with no runner (a third sibling fails, or a newer push/concurrency cancels the run), the loop's finished and not any(refused(job)...) stop fires and the queued shard ends up cancelled without any re-run — an outcome the "assessed before this hold" carve-outs (refused, held-in-setup) don't cover, and one a plain budget rescue previously prevented. Consider letting a held stuck job survive the deadline (re-adopt in-progress runs regardless of marker age, or treat the hold like the refusal path and finish it within a grace window), so a sibling that finishes late still gets its shard rescued.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/ci/owned_pool_rescue.py, line 582:

<comment>A stuck job held back by a mini-running sibling has no recovery path once the watch's deadline passes. The watch loop returns "stop: watch limit reached" with nothing cancelled, and later sweepers skip this run as soon as its marker passes SWEEP_MAX_AGE_SECONDS (150 min; sweep() drops marked runs whose `created` is older than `oldest`). If the pool stays busy past that point the shard waits indefinitely and is never rescued or re-run. The narrower new case this hold also enables: if the run reaches "completed" while the held job is still queued with no runner (a third sibling fails, or a newer push/concurrency cancels the run), the loop's `finished and not any(refused(job)...)` stop fires and the queued shard ends up cancelled without any re-run — an outcome the "assessed before this hold" carve-outs (refused, held-in-setup) don't cover, and one a plain budget rescue previously prevented. Consider letting a held stuck job survive the deadline (re-adopt in-progress runs regardless of marker age, or treat the hold like the refusal path and finish it within a grace window), so a sibling that finishes late still gets its shard rescued.</comment>

<file context>
@@ -566,6 +579,9 @@ def assess(jobs: Sequence[Mapping[str, Any]], *, now: dt.datetime, budget_second
     if turned_away:
         names = ", ".join(sorted(str(job.get("name") or job.get("id")) for job in turned_away))
         return Look("refused", f"{names} refused by {job_pool(turned_away[0])} at job start or Xcode selection")
+    if stuck:
+        return Look("watch", f"{names} queued on {job_pool(stuck[0])} with no runner, but "
+                             f"{len(on_mini)} job(s) of the run are running on a persistent runner", waiting=True)
</file context>

return Look("watch", f"{names} queued on {job_pool(stuck[0])} with no runner, but "
f"{len(on_mini)} job(s) of the run are running on a persistent runner", waiting=True)
if waiting or settling:
return Look("watch", f"{len(waiting)} job(s) waiting for a persistent runner, {len(settling)} in its setup",
waiting=True)
Expand Down
45 changes: 45 additions & 0 deletions tests/test_ci_owned_pool_rescue.py
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,51 @@ def test_a_stuck_side_job_moves_to_blacksmith_keeping_what_passed(self):
self.assertIn("attempt 2 takes the side lane's Blacksmith default", summary)
self.assertNotIn("jobs:2", api.calls)

def test_a_stuck_side_job_never_cancels_a_sibling_running_on_a_mini(self):
# #16463: cmux-next's swift test ran on a mini while release-compile waited
# for one; the rescue cancelled both and moved them to Blacksmith.
def cmux_next(done_at):
def jobs(seconds):
test = job("cmux-next swift test", labels=[SIDE], status="in_progress", runner="mini-5-glaeda-3")
test["started_at"] = stamp(5)
if done_at is not None and seconds >= done_at:
test.update(status="completed", conclusion="success")
return [test, job("cmux-next Release compile (Xcode 26)", labels=[SIDE], created=0)]
return jobs

payload = side_event(path=".github/workflows/cmux-next.yml")
clock = Clock()
api = FakeAPI(clock, cmux_next(done_at=None))
code, summary = run_main(api, clock, payload=payload)
self.assertEqual(code, 0)
self.assertNotIn("cancel", api.calls)
self.assertNotIn("rerun-failed", api.calls)
self.assertIn("watch limit reached", summary)

# Once the mini's job ends, cancelling the run touches only the stuck job.
clock = Clock()
api = FakeAPI(clock, cmux_next(done_at=600))
code, summary = run_main(api, clock, payload=payload)
self.assertEqual(code, 0)
self.assertIn("cancel", api.calls)
self.assertIn("rerun-failed", api.calls)
self.assertGreaterEqual(clock.seconds, 600)

def test_a_sibling_on_a_mini_does_not_hide_a_refusal_or_a_held_job(self):
running = job("macos / shard 1", status="in_progress", labels=[MINI], runner="mini-2")
running["started_at"] = stamp(5)
stuck = job("macos / shard 2", labels=[MINI], created=0)
late = START + dt.timedelta(seconds=44 + rescue.SETUP_WAIT_SECONDS)
look = rescue.assess([running, stuck], now=late, budget_seconds=90)
self.assertEqual((look.action, look.waiting), ("watch", True))
self.assertIn("running on a persistent runner", look.reason)
look = rescue.assess([running, stuck, refused_job("macos / shard 3")], now=late, budget_seconds=90)
self.assertEqual(look.action, "refused")
closing = late + dt.timedelta(seconds=rescue.END_MARGIN_SECONDS - 1)
look = rescue.assess([running, stuck, setup_job()], now=late, budget_seconds=90, deadline=closing)
self.assertEqual(look.action, "rescue")
self.assertIn("runner setup", look.reason)

def test_a_side_lane_retry_takes_blacksmith(self):
target = rescue.target_from_event(side_event(), "manaflow-ai/cmux")
self.assertIn("Blacksmith default", rescue.next_attempt(target))
Expand Down
Loading