diff --git a/.github/workflows/build_check_cache.yml b/.github/workflows/build_check_cache.yml index 3207ac140b..f9ba63a3e6 100644 --- a/.github/workflows/build_check_cache.yml +++ b/.github/workflows/build_check_cache.yml @@ -143,7 +143,6 @@ jobs: env: WAIT_MINUTES: 240 PRIORITY: ${{ contains(github.event.pull_request.labels.*.name, 'macos-priority') }} - ARCH: ${{ inputs.arch }} # Must exceed WAIT_MINUTES. timeout-minutes: 250 # One line for every arch of every pull request, and only its first job polls. diff --git a/scripts/ci_macos_admission.py b/scripts/ci_macos_admission.py index 42fe7a4de4..35453dde04 100644 --- a/scripts/ci_macos_admission.py +++ b/scripts/ci_macos_admission.py @@ -4,20 +4,20 @@ Runs from a Linux job in build_check_cache.yml, once per macOS arch, in a single repo-wide line (a concurrency group with queue: max), so only the job at the front of the line polls. It lets its arch in when the runners every active Build -all run holds or still needs, plus what letting this arch in adds to its own -run, fit in MACOS_RUNNER_LIMIT: +all run holds or still needs, plus one for this arch, fit in MACOS_RUNNER_LIMIT: - A push, nightly or manual run holds RESERVE runners until its macOS work is done. The jobs API lists only the jobs a run has reached, so what it still needs cannot be counted and is reserved instead. -- A pull request run with both arches let in holds RESERVE runners until its - macOS work is done, which covers its universal build and tests that never - pass through the line and start only after both arch builds. -- A pull request run with one arch let in holds one runner until that arch's +- A pull request run holds one runner for each arch let in until that arch's app build is done, and none after. A run holding a runner while it waits for its other arch would deadlock: the other arch can sit in the line behind a pull request that waits for that runner. - A run holds at least the macOS jobs it has queued or running. + +A pull request's universal build and tests skip the line and are not held for +in advance. They take minutes, and holding two runners for them through the +slower arch's build left runners idle while other builds waited. - In the minutes around the nightly's cron time, RESERVE runners are held for it until its run appears. @@ -28,8 +28,8 @@ waiting as holding a runner, so the next free runners go to it. Any API error repeated FAILURES_BEFORE_ADMIT times, and the WAIT_MINUTES limit, let the arch in, so a fault here never blocks pull requests. -Environment: GH_TOKEN, REPO, WORKFLOW_REF, GITHUB_RUN_ID, ARCH, and optionally -PRIORITY, MACOS_RUNNER_LIMIT, WAIT_MINUTES, POLL_SECONDS and GITHUB_API_URL. +Environment: GH_TOKEN, REPO, WORKFLOW_REF, GITHUB_RUN_ID, and optionally PRIORITY, +MACOS_RUNNER_LIMIT, WAIT_MINUTES, POLL_SECONDS and GITHUB_API_URL. """ import datetime @@ -41,8 +41,8 @@ import urllib.error import urllib.parse import urllib.request -# A Build all run builds arm64 and x86_64 at the same time, then the universal -# build and the macOS tests at the same time. +# A push, nightly or manual run builds arm64 and x86_64 at the same time, then +# the universal build and the macOS tests at the same time. RESERVE = 2 # Must match the job names in build_all.yml and build_check_cache.yml. @@ -97,10 +97,9 @@ def priority_waiting(jobs): return sum(1 for job in gates(jobs, (PRIORITY_GATE,)) if job["status"] != "completed") -def run_demand(run, jobs, yield_to_priority=False, letting_in=None): +def run_demand(run, jobs, yield_to_priority=False): """macOS runners a run holds or still needs. With yield_to_priority, each - priority arch still waiting counts as holding one. letting_in counts that arch - as let in.""" + priority arch still waiting counts as holding one.""" # A run with no jobs that is pending waits behind another run of its # concurrency group, which holds the runners for both. if not jobs and run["status"] in ("pending", "waiting"): @@ -110,10 +109,7 @@ def run_demand(run, jobs, yield_to_priority=False, letting_in=None): return active if run["event"] != "pull_request": return max(active, RESERVE) - arches = admitted_arches(jobs) | ({letting_in} if letting_in else set()) - if len(arches) >= RESERVE: - return max(active, RESERVE) - held = sum(1 for arch in arches if not arch_built(jobs, arch)) + held = sum(1 for arch in admitted_arches(jobs) if not arch_built(jobs, arch)) if yield_to_priority: held += priority_waiting(jobs) return max(active, held) @@ -162,11 +158,10 @@ def latest_run(api, repo, workflow, **params): return runs[0] if runs else None -def measure(api, repo, workflow, run_id, now, priority=False, arch=None): - """Total macOS runners held or needed, one line per run that holds any, and - how many more letting arch in adds to run_id. The priority line does not - count the priority runs waiting behind it.""" - total, lines, need = 0, [], 1 +def measure(api, repo, workflow, run_id, now, priority=False): + """Total macOS runners held or needed, and one line per run that holds any. + The priority line does not count the priority runs waiting behind it.""" + total, lines = 0, [] # A run is listed as queued whenever one of its jobs waits for a runner, and # as pending whenever one waits in a concurrency group, so runs in any of these # can hold runners. A run can move between the lists between the calls. @@ -177,8 +172,6 @@ def measure(api, repo, workflow, run_id, now, priority=False, arch=None): filter="latest", per_page=100)["jobs"] own = run["id"] == run_id demand = run_demand(run, jobs, yield_to_priority=not priority and not own) - if own: - need = max(1, run_demand(run, jobs, letting_in=arch) - demand) if demand: total += demand waiting = ", priority, waiting" if priority_waiting(jobs) and not own else "" @@ -191,7 +184,7 @@ def measure(api, repo, workflow, run_id, now, priority=False, arch=None): if not last or parse_time(last["created_at"]) < start: total += RESERVE lines.append(f" {RESERVE} the nightly, due at {NIGHTLY_UTC:%H:%M} UTC") - return total, lines, need + return total, lines def wait(measure_now, limit, wait_minutes, poll_seconds, @@ -201,7 +194,7 @@ def wait(measure_now, limit, wait_minutes, poll_seconds, failures = 0 while True: try: - total, lines, need = measure_now() + total, lines = measure_now() except (urllib.error.URLError, OSError, ValueError, KeyError, TypeError) as error: failures += 1 log(f"::warning title=macOS admission::Could not read the queue ({error}).") @@ -209,10 +202,10 @@ def wait(measure_now, limit, wait_minutes, poll_seconds, return "the queue could not be read" else: failures = 0 - log(f"{total} of {limit} macOS runners held or needed, and this arch needs {need}:") + log(f"{total} of {limit} macOS runners held or needed:") for line in lines: log(line) - if total + need <= limit: + if total + 1 <= limit: return "a runner is free" if clock() + poll_seconds >= deadline: return f"it waited {wait_minutes} minutes" @@ -227,7 +220,7 @@ def main(): limit = int(os.environ.get("MACOS_RUNNER_LIMIT") or 5) reason = wait( lambda: measure(api, repo, workflow, run_id, datetime.datetime.now(datetime.timezone.utc), - priority=os.environ.get("PRIORITY") == "true", arch=os.environ["ARCH"]), + priority=os.environ.get("PRIORITY") == "true"), limit, wait_minutes=int(os.environ.get("WAIT_MINUTES") or 240), poll_seconds=int(os.environ.get("POLL_SECONDS") or 180), diff --git a/scripts/tests/test_ci_macos_admission.py b/scripts/tests/test_ci_macos_admission.py index 65c9588cf0..c9f8ddee38 100644 --- a/scripts/tests/test_ci_macos_admission.py +++ b/scripts/tests/test_ci_macos_admission.py @@ -93,11 +93,17 @@ class RunDemandTest(unittest.TestCase): failed = job("build_macos_arch (arm64) / Build Deps / Build Deps", conclusion="failure") self.assertEqual(admission.run_demand(PR, [gate("arm64"), failed]), 0) - def test_pull_request_keeps_its_runners_for_its_later_jobs(self): - # Both arch builds are done and the universal build and tests are not - # listed yet: they will need both runners without passing the line. - jobs = [gate("arm64"), gate("x86_64"), build("arm64", "completed"), build("x86_64", "completed")] - self.assertEqual(admission.run_demand(PR, jobs), 2) + def test_both_arches_let_in_hold_only_what_still_builds(self): + # The universal build and tests are not held for in advance. + jobs = [gate("arm64"), gate("x86_64"), build("arm64", "completed"), build("x86_64")] + self.assertEqual(admission.run_demand(PR, jobs), 1) + + def test_universal_build_and_tests_count_once_queued(self): + built = [gate("arm64"), gate("x86_64"), build("arm64", "completed"), build("x86_64", "completed")] + self.assertEqual(admission.run_demand(PR, built), 0) + queued = [job("Build macOS Universal / Build OrcaSlicer", "queued", None), + job("macOS arm64 / Unit Tests", "in_progress", None)] + self.assertEqual(admission.run_demand(PR, built + queued), 2) def test_pull_request_releases_when_macos_is_done(self): jobs = [gate("arm64"), gate("x86_64"), UNIVERSAL_DONE, TESTS_DONE] @@ -138,16 +144,16 @@ class PriorityTest(unittest.TestCase): def test_measure_from_each_line(self): api = FakeApi({("runs", "pending"): [dict(PR, status="pending")], jobs_path(2): self.WAITING}) - total, lines, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) + total, lines = admission.measure(api, "o/r", "build_all.yml", 99, NOON) self.assertEqual(total, admission.RESERVE) self.assertIn("priority, waiting", lines[0]) - total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON, priority=True) + total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON, priority=True) self.assertEqual(total, 0) def test_own_waiting_run_is_not_counted(self): api = FakeApi({("runs", "pending"): [dict(PR, status="pending")], jobs_path(2): self.WAITING}) - total, _, need = admission.measure(api, "o/r", "build_all.yml", 2, NOON, arch="arm64") - self.assertEqual((total, need), (0, 1)) + total, _ = admission.measure(api, "o/r", "build_all.yml", 2, NOON) + self.assertEqual(total, 0) class FakeApi: @@ -182,7 +188,7 @@ class MeasureTest(unittest.TestCase): jobs_path(2): [gate("arm64"), build("arm64")], jobs_path(3): [gate("arm64"), gate("x86_64"), build("arm64", "queued")], }) - total, lines, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) + total, lines = admission.measure(api, "o/r", "build_all.yml", 99, NOON) self.assertEqual(total, admission.RESERVE + 1 + admission.RESERVE) self.assertEqual(len(lines), 3) @@ -190,25 +196,25 @@ class MeasureTest(unittest.TestCase): # One of its jobs waits in a concurrency group while its macOS builds run. pending_pr = dict(PR, status="pending") api = FakeApi({("runs", "pending"): [pending_pr], jobs_path(2): [gate("arm64"), build("arm64")]}) - total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) + total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) self.assertEqual(total, 1) def test_a_push_waiting_behind_another_holds_nothing(self): api = FakeApi({("runs", "pending"): [dict(PUSH, status="pending")], jobs_path(1): []}) - total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) + total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) self.assertEqual(total, 0) def test_reads_every_page_of_runs(self): runs = [dict(PR, id=i) for i in range(150)] responses = {("runs", "in_progress"): runs} responses.update({jobs_path(i): [gate("arm64")] for i in range(150)}) - total, _, _ = admission.measure(FakeApi(responses), "o/r", "build_all.yml", 999, NOON) + total, _ = admission.measure(FakeApi(responses), "o/r", "build_all.yml", 999, NOON) self.assertEqual(total, 150) def test_reserves_for_the_nightly_until_its_run_appears(self): due = datetime.datetime(2026, 10, 10, 2, 10, tzinfo=UTC) yesterday = {"created_at": "2026-10-09T02:20:00Z"} - total, lines, _ = admission.measure( + total, lines = admission.measure( FakeApi({("runs", "schedule"): [yesterday]}), "OrcaSlicer/OrcaSlicer", "build_all.yml", 99, due) self.assertEqual(total, admission.RESERVE) self.assertIn("nightly", lines[0]) @@ -216,13 +222,13 @@ class MeasureTest(unittest.TestCase): def test_no_nightly_reserve_once_its_run_exists(self): due = datetime.datetime(2026, 10, 10, 2, 30, tzinfo=UTC) today = {"created_at": "2026-10-10T02:21:00Z"} - total, _, _ = admission.measure( + total, _ = admission.measure( FakeApi({("runs", "schedule"): [today]}), "OrcaSlicer/OrcaSlicer", "build_all.yml", 99, due) self.assertEqual(total, 0) def test_no_nightly_reserve_in_a_fork(self): due = datetime.datetime(2026, 10, 10, 2, 10, tzinfo=UTC) - total, _, _ = admission.measure(FakeApi({}), "fork/OrcaSlicer", "build_all.yml", 99, due) + total, _ = admission.measure(FakeApi({}), "fork/OrcaSlicer", "build_all.yml", 99, due) self.assertEqual(total, 0) def test_counts_a_run_in_both_lists_once(self): @@ -231,25 +237,13 @@ class MeasureTest(unittest.TestCase): ("runs", "queued"): [dict(PUSH, status="queued")], jobs_path(1): [], }) - total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) + total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON) self.assertEqual(total, admission.RESERVE) - def own(self, jobs, arch): - api = FakeApi({("runs", "in_progress"): [PR], jobs_path(2): jobs}) - total, _, need = admission.measure(api, "o/r", "build_all.yml", 2, NOON, arch=arch) - return total, need - - def test_first_arch_needs_one(self): - self.assertEqual(self.own([gate("arm64", "in_progress", None)], "arm64"), (0, 1)) - - def test_second_arch_needs_one_while_the_first_builds(self): + def test_counts_its_own_run(self): jobs = [gate("arm64"), build("arm64"), gate("x86_64", "in_progress", None)] - self.assertEqual(self.own(jobs, "x86_64"), (1, 1)) - - def test_second_arch_needs_both_once_the_first_is_built(self): - # Letting it in makes the run hold both for its universal build and tests. - jobs = [gate("arm64"), build("arm64", "completed"), gate("x86_64", "in_progress", None)] - self.assertEqual(self.own(jobs, "x86_64"), (0, 2)) + api = FakeApi({("runs", "in_progress"): [PR], jobs_path(2): jobs}) + self.assertEqual(admission.measure(api, "o/r", "build_all.yml", 2, NOON)[0], 1) def test_a_run_whose_other_arch_waits_behind_this_one_does_not_block_it(self): # The line is A-arm64, B-arm64, A-x86_64. A's arm64 is built and its @@ -260,8 +254,8 @@ class MeasureTest(unittest.TestCase): jobs_path(1): [gate("arm64"), build("arm64", "completed"), gate("x86_64", "pending", None)], jobs_path(2): [gate("arm64", "in_progress", None)], }) - total, _, need = admission.measure(api, "o/r", "build_all.yml", 2, NOON, arch="arm64") - self.assertEqual((total, need), (0, 1)) + total, _ = admission.measure(api, "o/r", "build_all.yml", 2, NOON) + self.assertEqual(total, 0) def test_no_nightly_lookup_outside_its_window(self): api = FakeApi({}) @@ -278,8 +272,7 @@ class WaitTest(unittest.TestCase): result = next(results) if isinstance(result, Exception): raise result - total, need = result if isinstance(result, tuple) else (result, 1) - return total, [], need + return result, [] def sleep(seconds): now[0] += seconds @@ -294,8 +287,6 @@ class WaitTest(unittest.TestCase): def test_waits_while_full(self): self.assertEqual(self.run_wait([5, 6, 4]), ("a runner is free", 240)) - def test_waits_for_what_this_arch_needs(self): - self.assertEqual(self.run_wait([(4, 2), (3, 2)]), ("a runner is free", 120)) def test_lets_in_after_repeated_api_errors(self): error = urllib.error.URLError("rate limited")