Skip to content

fix(ci): measured per-file timeout budgets (3x p90) + 16 queue slices (t_bf25c6b2) - #1208

Closed
Kyzcreig wants to merge 3 commits into
mainfrom
ci/measured-file-timeout-t_bf25c6b2
Closed

Kyzcreig wants to merge 3 commits into
mainfrom
ci/measured-file-timeout-t_bf25c6b2

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Kanban: t_bf25c6b2 (related: t_9c66df3d makes the two slow files fast + retry; t_bd6d58c3 kanban_db import cost)

Why

13 of 14 failed merge_group runs on 2026-09-25 (15:00-20:30 PT) were a FIXED 300 s per-file timeout on two files, with 0 assertion failures: tests/hermes_cli/test_kanban_home_session.py and tests/tools/test_execution_flag_detection.py. Each kill ejected its queue entry and cancelled the builds behind it (70 success / 30 failure / 69 cancelled).

Measured over 3,200 real test-durations-slice-* artifacts (2026-09-25 12:00Z to 09-26 03:47Z): those files ran 75-300 s on single attempts, so a fixed 300 s is a coin flip.

What

  1. Measured per-file budget in scripts/run_tests_parallel.py: clamp(3 x p90, floor, cap). The floor is --file-timeout (default 300 s). The cap is max(floor, 900 s). The basis is the p90 of the file's last 20 main-branch samples plus its last cached duration. --generate-slices stamps file_timeouts (path=secs:..., only for files above the floor) into each slice row. The test job passes it with the new --file-timeouts. A file with no measured duration gets the 900 s cap (not proven to fit 300). Unmeasured files are stamped individually. With no duration data at all, the stamp is a single *=900 default. A passed stamp (CI always passes one, possibly empty) is authoritative, and unlisted files get the floor. The local no-stamp path (no --file-timeouts) keeps the caller's --file-timeout for unmeasured files, so the runner's kill self-tests (--file-timeout 8) still kill.
  2. Duration history: test_durations_history.json lives in its own cache entry (test-durations-history-*). generate restores it. save-durations appends each main run through --merge-duration-history. The existing LPT cache entry and its path are untouched.
  3. merge_group uses 16 slices, the same as pull_request (it was 8). With 8, queue slices carried twice the files, ran 10-12 min and loaded the runners. The hosted cap is 120 since 09-25.

Replay (same 3,200 artifacts, chronological, per file-run)

budget rule file-runs that exceeded it
fixed 300 s (today) 51
3 x last sample 12 (incl. kanban_home_session x4, execution_flag_detection x2)
3 x p90(last 20) (this PR) 6

All 6 residual runs are test_local_env_blocklist.py (normally 15-31 s, recorded 309-318 s) or test_kanban_home_cards.py once. These are hang-then-quick-retry sums, not slow files. The budget deliberately does not widen for them, and the existing one-shot retry already rescues them.

Caveat: recorded values above the timeout are retry sums (attempt 1 killed + attempt 2), so they are lower bounds on the first attempt's wall time.

Verification

  • tests/test_run_tests_parallel_file_budget.py: 24 passed (5 new for unmeasured files at the cap / * default / authoritative empty stamp). With the arm and runner routing suites, 86 passed.
  • Mutants:
    • runner ignores budgets + generate skips stamping: 4 RED
    • basis ignores history (last sample only): 2 RED
    • unmeasured -> floor: 5 RED
    • passed empty stamp not authoritative: 1 RED
  • ruff clean. actionlint shows only the 2 pre-existing ci.yaml:134-135 sparse-checkout findings, which are identical on base.

Rollout note

The history cache is empty until the first push: main save-durations after merge. Until then, budgets fall back to 3 x the single cached LPT duration. Files missing from that cache get 900 s. Nothing gets less than today's 300 s.

…6b2)

13 of 14 failed merge_group runs (2026-09-25 15:00-20:30 PT) were two
files crossing the FIXED 300 s per-file wall (0 assertion failures):
test_kanban_home_session.py and test_execution_flag_detection.py. Each
kill ejected the queue entry and cancelled the builds behind it.

- run_tests_parallel.py: per-file budget = clamp(3 x p90, floor, cap),
  floor = --file-timeout (300), cap = max(floor, 900). Basis = p90 of
  the file's recent samples (new test_durations_history.json) plus its
  last cached duration. --generate-slices stamps `file_timeouts`
  (path=secs, only files above the floor) into each slice row; the test
  job passes it via the new --file-timeouts flag.
- tests.yml: generate restores the history cache (own key/path, LPT
  cache untouched); save-durations appends each main run's durations,
  keeping the newest 20 per file.
- ci.yaml: merge_group runs 16 slices like pull_request (was 8).

Verified: tests/test_run_tests_parallel_file_budget.py 19 passed (+ arm
and runner routing suites, 86 total); mutants (runner ignores budgets /
generate skips stamping / basis ignores history) turn 4 and 2 tests RED.
Replayed over 399 real slice-duration artifacts: 3x last-sample would
still have killed 2 runs; 3x p90(last 20) killed 0.
… (t_bf25c6b2)

A file with no duration sample is not known to fit in 300 s; a false kill
ejects a merge-queue entry and cancels every build behind it. The budget
for an unmeasured file is now max(floor, 900).

- generate stamps unmeasured files at the cap; with no duration data at
  all it stamps a single '*=<cap>' default entry, not one per file.
- --file-timeouts default is None. When the flag is passed (CI always
  passes it, possibly ''), the stamp is authoritative: unlisted file ->
  '*' entry, else floor. The test jobs have no local cache, so falling
  through to it would have made every file unmeasured.

Verified: tests/test_run_tests_parallel_file_budget.py 23 passed.
Mutants: unmeasured->floor 5 RED; passed empty stamp not authoritative
1 RED. ruff clean.
@Kyzcreig Kyzcreig added the ci-reviewed CI-sensitive changes reviewed by maintainer label Sep 26, 2026
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: unspecified · gate: BYPASS: FleetReview daily budget exhausted (spent=600 paged=true), no terminal record possible today; Ace directed admin-merge of the queue-bottleneck fixes (21:06 PT) · why: GUARD for merge-queue ejections: per-file timeout budgets measured at 3x p90 instead of a flat 300s ceiling, 16 merge_group slices. Companion to root-cause #1183 (merged f94ff55). 0 conflicts vs main. · red-ci allowed [Review label gate / Review label gate,All required checks pass (required, ABSENT on head)]: slice 10/16 red = tests/discord/test_restart_backfill.py::test_backfill_first_boot_no_state_scans_nothing, not in diff; 17/17 green 2x on branch hermetic HOME; 3rd sighting today of this file flaking under xdist load (also hit #1182); Review label gate red = ci-reviewed label just applied

@blacksmith-sh

This comment has been minimized.

…files (t_bf25c6b2)

5c755ba gave every unmeasured file the 900 s cap on BOTH paths. The
runner's own kill self-tests (test_run_tests_parallel_timeout_verdict.py)
run it with --file-timeout 8 on an uncached hanging probe, so the probe
got 900 s, and the outer file hit its own 300 s budget: CI slice 4/16
TIMED OUT on run 36217271474.

The cap for unmeasured files now applies only on the stamped path, which
is what CI always runs. That is where the queue fix is needed. The local
no-stamp path keeps the caller's floor.

Verified: file_budget + timeout_verdict 35 passed; noop_guard +
run_tests_parallel 19 passed / 1 skipped; ruff clean.
@ang-fleet-workers

Copy link
Copy Markdown

Closing as superseded (rebase shard t_b88390a3). The fixed-300s per-file kill this PR budgets around no longer exists on main:

@ang-fleet-workers

Copy link
Copy Markdown

CLOSED: SUPERSEDED-BY #1212 — 294e745 (#1212) replaced the fixed 300s per-file kill with an idle hang detector + 1800s backstop; #1209 (edc262a) and #1117 (a504d10) cover retry and slice count; all ancestors of main. card t_9c66df3d by daedalus-opus (t_cef7a187)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-reviewed CI-sensitive changes reviewed by maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant