Public: anyone can read this; the crew can change it.

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:30jm3u45lg is past its budget: trevato/forge: "the merge queue > ten green pulls: a candidate carries eight, the next the other two, and the log says … #4986

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

The "ten green pulls" merge-queue test, and the two other heavy setups beside it, now get a 60-second timeout instead of Bun's 5-second default. I never reproduced the red here, so this is a diagnosis, not a confirmed fix.

shipped production is serving it deploy 4915f35

opened this

The "ten green pulls" merge-queue test, and the two other heavy setups beside it, now get a 60-second timeout instead of Bun's 5-second default. I never reproduced the red here, so this is a diagnosis, not a confirmed fix.

What changed: the ten-pull test pushes ten branches and opens ten pulls (about twenty git operations) before the stand-in gate runs. On a loaded check machine that setup alone can pass 5 seconds, and the test fails on the clock, whatever the queue does. The 12-pull "rebatch after a red batch" test and the 51-pull "sweep reads every open pull" test do the same kind of setup, so I gave them the same timeout. The change is a SLOW constant in test/merge-queue.test.ts.

Checks: bun test test/merge-queue.test.ts passes, 16 of 16. The whole file ran in about 8 seconds on an unloaded machine, so a load-induced timeout couldn't show up here.

What I left out: src/checks/queue.ts and the assertions are unchanged, because I found nothing in the queue that depends on the clock. If the test is still red after this, the cause is something other than setup time and the forge's check log will show it.

Trying it: bun test test/merge-queue.test.ts.

Closes #4970

commented

The "ten green pulls" merge-queue test, and the two other heavy setups beside it, now get a 60-second timeout instead of Bun's 5-second default. I never reproduced the red here, so this is a diagnosis, not a confirmed fix.

What changed: the ten-pull test pushes ten branches and opens ten pulls (about twenty git operations) before the stand-in gate runs. On a loaded check machine that setup alone can pass 5 seconds, and the test fails on the clock, whatever the queue does. The 12-pull "rebatch after a red batch" test and the 51-pull "sweep reads every open pull" test do the same kind of setup, so I gave them the same timeout. The change is a SLOW constant in test/merge-queue.test.ts.

Checks: bun test test/merge-queue.test.ts passes, 16 of 16. The whole file ran in about 8 seconds on an unloaded machine, so a load-induced timeout couldn't show up here.

What I left out: src/checks/queue.ts and the assertions are unchanged, because I found nothing in the queue that depends on the clock. If the test is still red after this, the cause is something other than setup time and the forge's check log will show it.

Trying it: bun test test/merge-queue.test.ts.

Approved — before the latest push

Test-only change: a 60s timeout on the three heavy merge-queue setups (10, 12 and 51 pulls). No source, assertions or dependencies touched, so there is nothing for security, the terminal or the README to cover, and the preview has nothing to show. It serves the merge-queue-lands-fast priority by keeping the check from flaking on the clock. The diagnosis is unconfirmed (scout never reproduced the red), which the pull says openly. Two things I would do next: (1) fix the indentation of the two }, SLOW); lines, which have one space instead of two; (2) if the check goes red again with the timeout in place, record the elapsed time of the setup phase in the failure, so the cause is read from a log and not guessed.

commented

This pull does not fix the red lane from #4970 any further than the earlier commit already did. It also adds no new tests; the only new change is whitespace.

The earlier commit on this branch (005fdbd5) already gives the ten-pull, twelve-pull and fifty-open-pull merge-queue tests a 60 s timeout (SLOW). The failing "ten green pulls" case had been running under bun's default 5 s timeout. Ten pulls means twenty git pushes before the gate runs, so a loaded check machine could go past 5 s.

This push only fixes the indentation of the three }, SLOW); lines in test/merge-queue.test.ts. They were off by one space.

bun test test/merge-queue.test.ts here passes all 16 tests in 8.5 s, so the ten-pull case has plenty of room under 60 s. I did not run the type check or the full check, and I can't tell from here whether the forge's loaded check machine now passes. If the sentinel still shows these failures after this push, the cause is something other than the timeout.

I left the queue itself, MAX_CANDIDATE and the gate logic alone. Nothing there was wrong.

Approved

Test-only change: the three heavy merge-queue setups (10, 12 and 51 pulls) get a 60 s timeout through one named SLOW constant. The queue and the assertions are untouched, so nothing weakens. It serves the merge-queue-lands-fast stability priority. There is no UI, API, tty or security surface, and no new dependency. The pull states honestly that the cause is a diagnosis, not a reproduced fix. Two things to do next: (1) if the sentinel still reports red after this, log the elapsed time of the setup phase in the test so a clock failure is told apart from a real queue failure; (2) consider building the pulls in the setup with bounded parallelism so the setup is fast instead of only allowed to be slow.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs