diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index 224f9b90ae83..217bee801ff8 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -606,6 +606,9 @@ jobs: id: owned-state if: steps.reuse-products.outputs.hit != 'true' && (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && startsWith(env.CMUX_PRODUCT_RUNNER, 'glaeda-') continue-on-error: true + env: + # This pull request's build parked on this root comes back first (owned_build_state.py PR slots). + CMUX_OWNED_PR: ${{ github.event.pull_request.number }} run: | set -euo pipefail fingerprint="$(scripts/ci/compile-app-host-test-product.sh canonical-fingerprint "$CMUX_COMPILE_ADMISSION_DERIVED_DATA")" diff --git a/docs/ci-runners.md b/docs/ci-runners.md index ed03d5c0e424..11235b98e818 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -315,6 +315,15 @@ loaded mini, then the name. The picker's candidates, pick and predicted seconds go to admission's record (`route.picker`) through the `admission_route` output. +When `keep` replaces another pull request's build, it parks that build in +`pr-builds/pr-` beside the root's store (a rename; at most 2 per root, for +6 h, and only with 140 GiB free). Admission's +`check` for that pull request (`CMUX_OWNED_PR`) swaps it back in, +glaeda's hook ranks the root by it, and `roots` publishes it as `parked`, so +distance routing sends a re-push to the mini holding its own build. That +start ranks far even when the pull request changes a package interface, +where every other start rebuilds the app. + The cost model is `scripts/ci/warm-distance-model.json`, fitted by `scripts/ci/warm_distance.py fit` from the line every owned admission appends to `/Users/Shared/cmux-build-fleet/ci/admissions.jsonl` on its mini (start, diff --git a/scripts/ci/owned_build_state.py b/scripts/ci/owned_build_state.py index 5e5c17611893..a730a8620c5f 100644 --- a/scripts/ci/owned_build_state.py +++ b/scripts/ci/owned_build_state.py @@ -163,6 +163,7 @@ import shutil import subprocess import sys +import time sys.path.insert(0, str(Path(__file__).resolve().parent)) import seed_derived_data as seed # noqa: E402 @@ -268,9 +269,152 @@ def sweep_discarded(store: Path) -> None: remove(stale) -def check(store: Path, fingerprint: str, workspace: Path, package_store: Path | None = None) -> dict[str, str]: +# Pull request build slots. A root's kept DerivedData is replaced by the next admission on it, so a +# pull request's build was usually gone when its next push arrived: of 290 owned admissions on +# 2026-09-26/27, 69 re-pushes rebuilt the app because of their own package or app changes, 53 of them +# on another mini than their previous push and 16 on the same one after another pull request's build +# replaced it. A start from the same pull request's build skipped the app rebuild 11 times in 19. +# When `keep` replaces the kept build of another pull request, it parks that build in +# STORE/pr-builds/pr- (a rename, no copy), and `check` for pull request n swaps it back in before +# anything else reads the kept state. glaeda's hook reads the parked stamp when it ranks roots, and +# `warm-keys` publishes it (`parked`) for pr_runner_pool.py's distance routing. A root keeps at most +# PR_SLOTS parked builds, each for PR_SLOT_HOURS, and parks none while the volume has less than +# MIN_FREE_GIB free, so parked builds never crowd the mini's disk. +PR_BUILDS = "pr-builds" +PR_SLOTS = 2 +PR_SLOT_HOURS = 6 +MIN_FREE_GIB = 140 + + +def pr_slot(store: Path, number: object) -> Path | None: + key = pr_key(number) + return store / PR_BUILDS / key if key else None + + +def free_gib(path: Path) -> float: + try: + return shutil.disk_usage(path).free / 1024**3 + except OSError: + return 0.0 + + +def prune_pr_slots(store: Path, now: float | None = None) -> None: + """Drop parked builds past PR_SLOT_HOURS, then all but the newest PR_SLOTS.""" + now = time.time() if now is None else now + try: + entries = list((store / PR_BUILDS).iterdir()) + except OSError: + return + for path in entries: # a park or clear a killed job left half done + if (path.name.startswith(".pr-") and (".incoming-" in path.name or ".discard-" in path.name) + and not owned_by_live_process(path)): + slot = store / PR_BUILDS / path.name[1:].split(".", 1)[0] + with contextlib.suppress(OSError, RuntimeError): + # A park killed after its stamp was written is whole: finish it rather than lose the build. + if (".incoming-" in path.name and (path / DERIVED).is_dir() and not slot.exists() + and pr_key(read_stamp(path).get("pr")) == slot.name): + path.rename(slot) + else: + clear(path) + try: + entries = list((store / PR_BUILDS).iterdir()) + except OSError: + return + slots = [path for path in entries if path.name.startswith("pr-")] + dated = [] + for path in slots: + try: + dated.append((path.stat().st_mtime, path)) + except OSError: + continue + dated.sort(reverse=True) + for index, (moment, path) in enumerate(dated): + if index >= PR_SLOTS or now - moment > PR_SLOT_HOURS * 3600: + with contextlib.suppress(OSError, RuntimeError): + clear(path) + + +def park(store: Path) -> str: + """Move the kept build of a pull request into its slot; returns the slot name or "".""" + stamp = read_stamp(store) + slot = pr_slot(store, stamp.get("pr")) + if slot is None or not (store / DERIVED).is_dir() or not str(stamp.get("fingerprint") or "").endswith(STATE_VERSION): + return "" + if free_gib(store) < MIN_FREE_GIB: + return "" + incoming = slot.with_name(f".{slot.name}.incoming-{os.getpid()}") + remove(incoming) + incoming.mkdir(parents=True) + write_stamp(incoming, stamp) + write_stamp(store, {}) # the store no longer holds that build, whatever happens next + try: + (store / DERIVED).rename(incoming / DERIVED) + except OSError: + write_stamp(store, stamp) + raise + clear(slot) + incoming.rename(slot) + os.utime(slot) + return slot.name + + +def unpark(store: Path, number: object, fingerprint: str) -> bool: + """Swap pull request NUMBER's parked build in as the kept one, parking the current kept build first.""" + slot = pr_slot(store, number) + if slot is None or not (slot / DERIVED).is_dir(): + return False + try: + if time.time() - slot.stat().st_mtime > PR_SLOT_HOURS * 3600: + return False # expired: neither published nor routed to + except OSError: + return False + stamp = read_stamp(slot) + if not fingerprint or stamp.get("fingerprint") != stamped(fingerprint): + return False + current = read_stamp(store) + if pr_key(current.get("pr")) == pr_key(number) and (store / DERIVED).is_dir(): + return False # the kept build is this pull request's already + # A current main build (no pull request) stays, and so does one a park refused for disk: the job + # starts from it instead. A kept build no router can read (stale or missing stamp) is replaced. + kept_current = str(current.get("fingerprint") or "").endswith(f"-{STATE_VERSION}") + if (store / DERIVED).exists() and kept_current and not park(store): + return False + if (store / DERIVED).exists(): + clear(store / DERIVED) + (slot / DERIVED).rename(store / DERIVED) + write_stamp(store, stamp) + with contextlib.suppress(OSError, RuntimeError): + clear(slot) + return True + + +def parked_stamps(store: Path, now: float | None = None) -> list[dict[str, object]]: + """The current (STATE_VERSION, under PR_SLOT_HOURS) stamps of the builds parked beside STORE, newest first.""" + now = time.time() if now is None else now + try: + dated = sorted(((path.stat().st_mtime, path) for path in (store / PR_BUILDS).iterdir() + if path.name.startswith("pr-") and (path / DERIVED).is_dir()), reverse=True) + except OSError: + return [] + slots = [path for moment, path in dated if now - moment <= PR_SLOT_HOURS * 3600] + found = [] + for slot in slots: + stamp = read_stamp(slot) + if str(stamp.get("fingerprint") or "").endswith(f"-{STATE_VERSION}") and pr_key(stamp.get("pr")) == slot.name: + found.append(stamp) + return found + + +def check(store: Path, fingerprint: str, workspace: Path, package_store: Path | None = None, + pr_number: str = "") -> dict[str, str]: store.mkdir(parents=True, exist_ok=True) sweep_discarded(store) + unparked = False + if pr_number: + try: + unparked = unpark(store, pr_number, fingerprint) + except (OSError, RuntimeError): + unparked = False if fingerprint and os.environ.get("RUNNER_OS") and os.environ.get("RUNNER_ARCH"): # Which seeds this root adopts, for seed_derived_data.py `prefetch` # to fetch ahead while the Mac is idle. Best effort. @@ -302,7 +446,7 @@ def check(store: Path, fingerprint: str, workspace: Path, package_store: Path | result["reason"] = f"kept DerivedData grew to {size} bytes" else: result["warm"] = "true" - result["reason"] = "kept DerivedData matches" + result["reason"] = "this pull request's parked build" if unparked else "kept DerivedData matches" packages = (package_store or store) / PACKAGES if packages.is_dir(): destination = workspace / ".ci-source-packages" @@ -399,6 +543,18 @@ def keep(store: Path, derived: Path, fingerprint: str, merged_onto: str = "", pr # A seed's record is never replayed here (adopt reads RECORD only). for name in (*UNREAD, seed.MANIFEST): remove(incoming / name) + # Another pull request's build is parked, not dropped: its next push may come back to it. + parked = "" + if pr_key(read_stamp(store).get("pr")) != pr_key(pr_number): + try: + parked = park(store) + except (OSError, RuntimeError): + parked = "" + own_slot = pr_slot(store, pr_number) + if own_slot is not None: # this build supersedes any parked build of the same pull request + with contextlib.suppress(OSError, RuntimeError): + clear(own_slot) + prune_pr_slots(store) stamp = read_stamp(store) stamp.pop("fingerprint", None) stamp.pop("merged_onto", None) @@ -415,7 +571,7 @@ def keep(store: Path, derived: Path, fingerprint: str, merged_onto: str = "", pr if pr_key(pr_number): stamp["pr"] = int(pr_number.strip()) write_stamp(store, stamp) - return {"kept": "true"} + return {"kept": "true", **({"parked": parked} if parked else {})} def pr_key(number: object) -> str: @@ -462,14 +618,18 @@ def root_number(store: Path, first: Path) -> int: def root_summary(store: Path, number: int) -> dict[str, object]: - """What glaeda's hook reads from root NUMBER's stamp, when it keeps a current build; else the number alone.""" + """What glaeda's hook reads from root NUMBER's stamp, when it keeps a current build, and from its parked + pull request builds (`parked`); else the number alone.""" stamp = read_stamp(store) - if not (store / DERIVED).is_dir() or not str(stamp.get("fingerprint") or "").endswith(f"-{STATE_VERSION}"): - return {"root": number} summary: dict[str, object] = {"root": number} - for field in ROOT_FIELDS: - if field in stamp: - summary[field] = stamp[field] + if (store / DERIVED).is_dir() and str(stamp.get("fingerprint") or "").endswith(f"-{STATE_VERSION}"): + for field in ROOT_FIELDS: + if field in stamp: + summary[field] = stamp[field] + # The pull request builds parked beside it (PR slots), which `check` swaps in for their next push. + parked = [{field: slot[field] for field in ROOT_FIELDS if field in slot} for slot in parked_stamps(store)] + if parked: + summary["parked"] = parked[:PR_SLOTS] return summary @@ -826,7 +986,8 @@ def package_store(argv: list[str]) -> Path | None: def main(argv: list[str]) -> int: if len(argv) in (5, 6) and argv[1] == "check": - write_outputs(check(Path(argv[2]), argv[3], Path(argv[4]), package_store(argv))) + write_outputs(check(Path(argv[2]), argv[3], Path(argv[4]), package_store(argv), + os.environ.get("CMUX_OWNED_PR", ""))) return 0 if len(argv) == 5 and argv[1] == "adopt": write_outputs(adopt(Path(argv[2]), Path(argv[3]), Path(argv[4]).resolve())) diff --git a/scripts/ci/owned_warm_state.py b/scripts/ci/owned_warm_state.py index d82c0d684891..4bcec5702133 100644 --- a/scripts/ci/owned_warm_state.py +++ b/scripts/ci/owned_warm_state.py @@ -92,6 +92,7 @@ # Roots per mini and files per root kept (owned_build_state.py MAX_PATHS caps the stamp's list). MAX_ROOTS = 4 MAX_ROOT_FILES = 400 +MAX_PARKED = 2 # parked pull request builds per root (owned_build_state.py PR_SLOTS) MAX_AGE_HOURS = 24 MAX_JOB_PAGES = 3 # Previous snapshots read, newest first, for the last one that has `warm`. @@ -120,8 +121,31 @@ def keys(document: Any) -> list[str]: return found[:MAX_KEYS] +def stamp_fields(entry: Mapping[str, Any]) -> dict[str, Any]: + """A root's or parked build's stamp fields (owned_build_state.py ROOT_FIELDS), each checked.""" + clean: dict[str, Any] = {} + onto = str(entry.get("merged_onto") or "").lower() + if len(onto) == 40 and warm_key(onto): + clean["merged_onto"] = onto + pr = entry.get("pr") + if isinstance(pr, int) and not isinstance(pr, bool) and 0 < pr < 10**9: + clean["pr"] = pr + files = entry.get("pr_app_swift_files") + if isinstance(files, list): + clean["pr_app_swift_files"] = [path for path in files[:MAX_ROOT_FILES] + if isinstance(path, str) and 0 < len(path) <= 512 and "\0" not in path] + total = entry.get("pr_app_swift_total") + if isinstance(total, int) and not isinstance(total, bool) and 0 <= total < 10**6: + clean["pr_app_swift_total"] = total + if entry.get("pr_package_interface") in (True, False, None) and "pr_package_interface" in entry: + clean["pr_package_interface"] = entry.get("pr_package_interface") + return clean + + def roots(document: Any) -> list[dict[str, Any]]: - """The artifact's valid roots (owned_build_state.py `warm-keys`), at most MAX_ROOTS, fields checked.""" + """The artifact's valid roots (owned_build_state.py `warm-keys`), at most MAX_ROOTS, fields checked. + + A root's `parked` pull request builds (at most MAX_PARKED, each with a pull request) are kept too.""" raw = document.get("roots") if isinstance(document, Mapping) else None found: list[dict[str, Any]] = [] seen: set[int] = set() @@ -130,22 +154,13 @@ def roots(document: Any) -> list[dict[str, Any]]: if not isinstance(number, int) or isinstance(number, bool) or not 0 < number < 100 or number in seen: continue seen.add(number) - clean: dict[str, Any] = {"root": number} - onto = str(entry.get("merged_onto") or "").lower() - if len(onto) == 40 and warm_key(onto): - clean["merged_onto"] = onto - pr = entry.get("pr") - if isinstance(pr, int) and not isinstance(pr, bool) and 0 < pr < 10**9: - clean["pr"] = pr - files = entry.get("pr_app_swift_files") - if isinstance(files, list): - clean["pr_app_swift_files"] = [path for path in files[:MAX_ROOT_FILES] - if isinstance(path, str) and 0 < len(path) <= 512 and "\0" not in path] - total = entry.get("pr_app_swift_total") - if isinstance(total, int) and not isinstance(total, bool) and 0 <= total < 10**6: - clean["pr_app_swift_total"] = total - if entry.get("pr_package_interface") in (True, False, None) and "pr_package_interface" in entry: - clean["pr_package_interface"] = entry.get("pr_package_interface") + clean: dict[str, Any] = {"root": number, **stamp_fields(entry)} + parked = entry.get("parked") + parked = [stamp_fields(item) for item in parked[:MAX_PARKED] if isinstance(item, Mapping)] \ + if isinstance(parked, list) else [] + parked = [item for item in parked if "pr" in item] + if parked: + clean["parked"] = parked found.append(clean) return sorted(found, key=lambda item: item["root"])[:MAX_ROOTS] diff --git a/scripts/ci/warm_distance.py b/scripts/ci/warm_distance.py index eb22dc3ac62f..1bcee752db9a 100644 --- a/scripts/ci/warm_distance.py +++ b/scripts/ci/warm_distance.py @@ -681,9 +681,10 @@ def hook_root_cost(changes: tuple[set[str], bool] | None, stamp: Mapping[str, An STAMP the root's kept build (None: none, the cold cost), NUMBER the job's pull request, MODEL hook_model()'s. OWN (the job's own `paths` and features()) is what the hook leaves out as the same for every root; with it the seconds are the whole job's prediction rather than a lower bound. A kept build of the same pull - request has those files already, so there they count only for a package interface or hot file, which a - re-push usually touches again (14 of 23 such starts rebuilt on 2026-09-26/27). Without OWN this is the - hook's cost.""" + request has those files already, so there they count only for a hot file, or a package interface, which a + re-push often touches again (14 of 23 such starts rebuilt on 2026-09-26/27). That start still beats every + other one for a job with its own package interface change (they all rebuild), so it ranks far, not + rebuild. Without OWN this is the hook's cost.""" if stamp is None: return max(model["tiers"].values()) + 1.0, "cold", -1 same = isinstance(stamp.get("pr"), int) and stamp.get("pr") == number @@ -701,14 +702,18 @@ def hook_root_cost(changes: tuple[set[str], bool] | None, stamp: Mapping[str, An package = package or (kept_package and interface is not False) changed = files | kept_set own_paths: set[str] = set() + own_package = False if own is not None: own_paths = {str(path) for path in own.get("paths") or [] if isinstance(path, str) and app_swift(path)} - package = package or bool(own.get("package_swift_files") and own.get("package_interface") is not False) + own_package = bool(own.get("package_swift_files") and own.get("package_interface") is not False) if not same: # a kept build of this pull request has its files already: they count only as a kind changed |= own_paths + package = package or own_package count = len(changed) + extra hot = bool((changed | own_paths) & set(model["hot_files"])) name = ("rebuild" if package or hot else "far" if count > model["near_app_swift_files"] else "near") + if same and own_package and name == "near": + name = "far" # the kept build has the package change unless this push touched it again # Every root starts from its kept build: that tier's kept-start p50 when the model has one. return (model.get("kept") or {}).get(name, model["tiers"][name]), name, count @@ -746,6 +751,13 @@ def mini_roots(warm: Mapping[str, Any], member: Callable[[str], str]) -> dict[st return {mini: roots for mini, (_, roots) in newest.items()} +def own_parked(entry: Mapping[str, Any], pr_number: int | None) -> list[Mapping[str, Any]]: + """A root's parked builds (owned_build_state.py PR slots) of pull request PR_NUMBER.""" + parked = entry.get("parked") + return [item for item in parked if isinstance(item, Mapping) and pr_number is not None + and item.get("pr") == pr_number] if isinstance(parked, list) else [] + + def distance_route(runners: Sequence[Mapping[str, Any]], root: str, *, minis: Mapping[str, Sequence[Mapping[str, Any]]], changes: Callable[[str], tuple[set[str], bool] | None], pr_number: int | None, @@ -756,7 +768,7 @@ def distance_route(runners: Sequence[Mapping[str, Any]], root: str, *, Every online `root` runner is a candidate. Its mini's roots (MINIS, the stamps admission publishes) each cost hook_root_cost() with the job's OWN - files. The hook hands the job the cheapest free root and each busy root + files, or by its parked build of this pull request, which admission swaps in. The hook hands the job the cheapest free root and each busy root runner of the mini holds one, so a runner costs the root ranked after the busy ones (the cheapest on an idle mini; for a busy runner, the one its job frees). A mini without stamps costs LEGACY[runner] (route_admission()'s @@ -792,6 +804,12 @@ def distance_route(runners: Sequence[Mapping[str, Any]], root: str, *, stamp = entry if entry.get("merged_onto") or entry.get("pr") else None cost = hook_root_cost(changes(str(entry.get("merged_onto") or "")) if stamp else None, stamp, pr_number, hook, own) + # This pull request's build parked beside the root: admission's `check` swaps it in (and the + # hook ranks the root by it) unless the kept build is a main build, which `check` never parks. + parked = own_parked(entry, pr_number) + if parked and (stamp is None or entry.get("pr")): + cost = hook_root_cost(changes(str(parked[0].get("merged_onto") or "")), parked[0], pr_number, + hook, own) costs.append((*cost, number if isinstance(number, int) and not isinstance(number, bool) else 0)) ranked[mini] = sorted(costs, key=lambda item: (item[0], item[2], item[3])) rows: list[dict[str, Any]] = [] @@ -860,8 +878,12 @@ def picker_distance_route(runners: Sequence[Mapping[str, Any]], root: str, *, me _deadline[0] = time.monotonic() + DISTANCE_BUDGET_SECONDS try: own_files = pull_request_files(workspace, base, fetch=False) if base else None + parked_bases = {str(item.get("merged_onto") or "") for roots in minis.values() for entry in roots + for item in own_parked(entry, number)} bases = sorted({str(entry.get("merged_onto") or "") for roots in minis.values() for entry in roots} - - {"", base})[:MAX_ROUTE_BASES] + - {"", base} - parked_bases) + # This pull request's parked bases first, so the cap never drops them. + bases = [*sorted(parked_bases - {"", base}), *bases][:MAX_ROUTE_BASES] if WARM_SHA.fullmatch(base): if bases: fetch_bases(workspace, bases) diff --git a/tests/test_ci_owned_build_state.py b/tests/test_ci_owned_build_state.py index 18502f1ecf32..9f079ce63198 100644 --- a/tests/test_ci_owned_build_state.py +++ b/tests/test_ci_owned_build_state.py @@ -40,6 +40,10 @@ def setUp(self): self.source = base / "canonical" / "src" self.packages = self.source / ".ci-source-packages" self.workspace.mkdir() + # Parking (PR slots) needs free disk; off unless a test turns it on, so the host's disk never matters. + self.free = unittest.mock.patch("owned_build_state.free_gib", return_value=0.0) + self.free.start() + self.addCleanup(self.free.stop) def tearDown(self): self.tmp.cleanup() @@ -311,6 +315,115 @@ def test_roots_carry_what_the_hook_reads_from_every_stamp(self): # Listed from root 2's store, the same roots. self.assertEqual(state.warm_keys(other, "r", "p")["roots"], roots) + def build(self, marker): + """A fresh compile of the DerivedData to keep, told apart by MARKER.""" + (self.derived / "Build").mkdir(parents=True, exist_ok=True) + (self.derived / "Build" / "marker").write_text(marker) + + def kept_marker(self, store=None): + return ((store or self.store) / "derived-data" / "Build" / "marker").read_text() + + def test_keep_parks_another_pull_requests_build_and_check_swaps_it_back(self): + with unittest.mock.patch("owned_build_state.free_gib", return_value=500.0): + self.build("seven") + self.kept(merged_onto=A, pr="7") + self.build("nine") + self.assertEqual(self.kept(merged_onto=B, pr="9"), {"kept": "true", "parked": "pr-7"}) + self.assertEqual(self.kept_marker(), "nine") + slot = self.store / "pr-builds" / "pr-7" + self.assertEqual(json.loads((slot / "stamp.json").read_text())["pr"], 7) + # warm-keys lists the parked build's pull request and publishes its stamp for the picker. + listed = self.keys(cache=False) + self.assertEqual(listed["keys"], ["b" * 12, "pr-9"]) # parked builds ride in `roots` only + self.assertEqual(listed["roots"], [{"root": 1, "merged_onto": B, "pr": 9, + "parked": [{"merged_onto": A, "pr": 7}]}]) + # Pull request 7's next push swaps its build back in and parks 9's. + result = run(state.check, self.store, "fp", self.workspace, None, "7") + self.assertEqual((result["warm"], result["reason"]), ("true", "this pull request's parked build")) + self.assertEqual(self.kept_marker(), "seven") + self.assertEqual(json.loads((self.store / "stamp.json").read_text())["pr"], 7) + self.assertFalse(slot.exists()) + self.assertTrue((self.store / "pr-builds" / "pr-9" / "derived-data").is_dir()) + # A re-push of the kept pull request replaces its build in place, parking nothing. + self.build("seven again") + self.assertEqual(self.kept(merged_onto=A, pr="7"), {"kept": "true"}) + + def test_check_never_drops_a_main_build_for_a_parked_one(self): + with unittest.mock.patch("owned_build_state.free_gib", return_value=500.0): + self.build("seven") + self.kept(pr="7") + self.build("main") + self.kept(pr="") # idle warming keeps main: 7 stays parked, main cannot be + result = run(state.check, self.store, "fp", self.workspace, None, "7") + self.assertEqual(result["reason"], "kept DerivedData matches") + self.assertEqual(self.kept_marker(), "main") + self.assertTrue((self.store / "pr-builds" / "pr-7" / "derived-data").is_dir()) + + def test_check_replaces_an_unreadable_kept_build_and_skips_an_expired_slot(self): + with unittest.mock.patch("owned_build_state.free_gib", return_value=500.0): + self.build("seven") + self.kept(pr="7") + self.build("nine") + self.kept(pr="9") + (self.store / "stamp.json").write_text("{}") + slot = self.store / "pr-builds" / "pr-7" + os.utime(slot, (1, 1)) + run(state.check, self.store, "fp", self.workspace, None, "7") + self.assertEqual(self.kept_marker(), "nine") + os.utime(slot) + result = run(state.check, self.store, "fp", self.workspace, None, "7") + self.assertEqual((result["reason"], self.kept_marker()), ("this pull request's parked build", "seven")) + + def test_keep_drops_a_stale_parked_build_of_its_own_pull_request(self): + with unittest.mock.patch("owned_build_state.free_gib", return_value=500.0): + self.build("seven") + self.kept(pr="7") + self.build("nine") + self.kept(pr="9") + self.build("seven, cold") + self.kept(pr="7") # say its check could not unpark: the new build supersedes the parked one + self.assertFalse((self.store / "pr-builds" / "pr-7").exists()) + self.assertEqual(self.kept_marker(), "seven, cold") + + def test_a_park_killed_after_its_stamp_is_finished(self): + whole = self.store / "pr-builds" / ".pr-5.incoming-999999999" + (whole / "derived-data").mkdir(parents=True) + (whole / "stamp.json").write_text(json.dumps({"pr": 5})) + torn = self.store / "pr-builds" / ".pr-6.incoming-999999998" + torn.mkdir() + state.prune_pr_slots(self.store) + self.assertEqual(sorted(path.name for path in (self.store / "pr-builds").iterdir()), ["pr-5"]) + + def test_check_leaves_a_parked_build_of_another_fingerprint(self): + with unittest.mock.patch("owned_build_state.free_gib", return_value=500.0): + self.build("seven") + self.kept(pr="7") + self.build("nine") + self.kept(pr="9") + result = run(state.check, self.store, "other-xcode", self.workspace, None, "7") + self.assertNotEqual(result["reason"], "this pull request's parked build") + self.assertEqual(self.kept_marker(), "nine") + self.assertTrue((self.store / "pr-builds" / "pr-7").is_dir()) + + def test_no_parking_below_the_free_disk_floor(self): + self.build("seven") + self.kept(pr="7") + self.build("nine") + self.assertEqual(self.kept(pr="9"), {"kept": "true"}) + self.assertFalse((self.store / "pr-builds" / "pr-7").exists()) + + def test_parked_builds_are_capped_by_count_and_age(self): + with unittest.mock.patch("owned_build_state.free_gib", return_value=500.0): + for number in ("1", "2", "3", "4"): + self.build(number) + self.kept(pr=number) + self.assertEqual(sorted(path.name for path in (self.store / "pr-builds").iterdir()), ["pr-2", "pr-3"]) + stale = self.store / "pr-builds" / "pr-2" + os.utime(stale, (1, 1)) + (self.store / "pr-builds" / ".pr-5.incoming-999999999").mkdir() + state.prune_pr_slots(self.store) + self.assertEqual(sorted(path.name for path in (self.store / "pr-builds").iterdir()), ["pr-3"]) + def test_at_most_eight_keys_without_repeats(self): self.kept() commits = (A, B, C, D, E, *(f"{digit}" * 40 for digit in range(5))) diff --git a/tests/test_ci_owned_warm_state.py b/tests/test_ci_owned_warm_state.py index 279def030e09..29b1c67e9c93 100644 --- a/tests/test_ci_owned_warm_state.py +++ b/tests/test_ci_owned_warm_state.py @@ -213,6 +213,14 @@ def test_roots_keep_only_the_fields_the_hook_reads_checked(self): [{"root": k} for k in range(1, state.MAX_ROOTS + 1)]) self.assertEqual(state.roots({}), []) + def test_roots_keep_parked_builds_with_a_pull_request(self): + document = {"roots": [{"root": 1, "pr": 9, "parked": [ + {"pr": 7, "merged_onto": SHA_A, "pr_package_interface": True, "fingerprint": "x"}, + {"merged_onto": SHA_B}, "junk", {"pr": 8}, {"pr": 6}]}, {"root": 2, "parked": "junk"}]} + self.assertEqual(state.roots(document), [ + {"root": 1, "pr": 9, "parked": [{"merged_onto": SHA_A, "pr": 7, "pr_package_interface": True}]}, + {"root": 2}]) + def test_fold_carries_the_roots_of_each_runners_newest_admission(self): mini = [{"root": 1, "merged_onto": SHA_A}] previous = {"through": 5, "runners": {"r1": {"keys": [A], "at": "2026-09-25T10:00:00Z", "roots": mini}}} diff --git a/tests/test_ci_warm_distance.py b/tests/test_ci_warm_distance.py index 49de56247d9d..bff46dd96f05 100644 --- a/tests/test_ci_warm_distance.py +++ b/tests/test_ci_warm_distance.py @@ -369,6 +369,9 @@ def test_the_job_own_files_join_the_distance(self): self.assertEqual(wd.hook_root_cost((swift(1, 2), False), stamp, 7, model, own), (266.5, "far", 6)) package = {"paths": [PACKAGE], "package_swift_files": 1, "package_interface": None} self.assertEqual(wd.hook_root_cost((set(), False), stamp, 7, model, package)[1], "rebuild") + # This pull request's own build has its package change already, unless this push touched it again. + self.assertEqual(wd.hook_root_cost((set(), False), {**stamp, "pr": 7}, 7, model, package)[1], "far") + self.assertEqual(wd.hook_root_cost((set(), True), {**stamp, "pr": 7}, 7, model, package)[1], "rebuild") def test_model_and_changes(self): self.assertEqual(wd.hook_model(HOOK_MODEL), {"near_app_swift_files": 5, "hot_files": ["Sources/Hot.swift"], @@ -393,7 +396,7 @@ def test_against_the_hook_itself_when_at_hand(self): for (changes, stamp, number), _ in HOOK_CASES: # A second, comparable root, so the hook never falls back to the exact keys. stamps = [stamp, {"merged_onto": "c" * 40}] - with unittest.mock.patch.object(hook, "root_stamp", lambda k, _dir="": stamps[k - 1]), \ + with unittest.mock.patch.object(hook, "root_stamp", lambda k, *_: stamps[k - 1]), \ unittest.mock.patch.object(hook, "main_changes", lambda _mirror, old, _new: changes if old == HOOK_BASE else (set(), False)): _, predicted = hook.warm_root_costs([0, 1], "d" * 40, number, tmp) @@ -460,6 +463,22 @@ def test_a_mini_without_stamps_costs_its_exact_key_or_the_unknown_start(self): "m3-glaeda": ("unknown", 309.7)}) self.assertEqual(name, "m1-glaeda") + def test_this_pull_requests_parked_build_draws_its_package_change_to_its_mini(self): + # Pull request 7 changes a package interface, so every other build rebuilds the app. + own = {"paths": [PACKAGE], "package_swift_files": 1, "package_interface": None} + parked = {"merged_onto": "1" * 40, "pr": 7, "pr_app_swift_files": [], "pr_app_swift_total": 0, + "pr_package_interface": True} + minis = {"m1": [self.stamp("1" * 40)], "m2": [{**self.stamp("1" * 40), "parked": [parked]}], + "m3": [{**self.stamp("1" * 40), "parked": [{**parked, "pr": 8}]}]} + changes = {"1" * 40: (set(), False)} + runners = [runner("m1-glaeda"), runner("m2-glaeda"), runner("m3-glaeda")] + name, decision = self.route(runners, minis, changes, own=own) + costs = {row["runner"]: row["tier"] for row in decision["candidates"]} + self.assertEqual(costs, {"m1-glaeda": "rebuild", "m2-glaeda": "far", "m3-glaeda": "rebuild"}) + self.assertEqual(name, "m2-glaeda") + self.assertEqual(wd.own_parked(minis["m2"][0], 7), [parked]) + self.assertEqual(wd.own_parked(minis["m2"][0], None), []) + def test_record_is_bounded_and_reads_either_mode(self): minis = {"m1": [self.stamp("1" * 40)]} _, decision = self.route([runner("m1-glaeda")], minis, {"1" * 40: (swift(1), False)})