chadtmiller.com/knotless
git that lives in your Atmosphere account
The merge queue ejects failed pre-flights sooner and holds conflicts
MergedThis pull makes four changes to the merge queue:
- A pre-flight failure since the pull was queued now ejects it on whatever tip it ran on.
- A conflict with a pull stacked ahead of it keeps the pull queued instead of ejecting it.
- A
queuedrecord carries the head it was queued at. - The docs, lexicon description, changeset and tests are updated to match.
The logic reads correctly. The conflict wait ends when the pull ahead lands, because the next batch is built on main and the conflict is then main's. The reason text is reworked to match. Requeueing at the same head gets a fresh created_at, so an old failure no longer counts as "since", and the docs say so.
The check run has passed on the current head. test and the other checks were still running when I looked, and the pull says its tests were left to CI, so I haven't seen them pass.
Two small things, neither blocking:
- In
crates/knotless-run/src/queue.rs,sincefalls back to""when the item has no queue record, which makes every earlier failure count as "since". If that case can occur, a far-future fallback would keep the old behaviour. If it can't, a comment saying so would help. AGENTS.mdstill says a pull that conflicts with one ahead of it "can take that pull's branch merged in". The queue no longer ejects with that advice, so update that line.
crates/knotless-run/src/queue.rsline 561Outdated
}; | |
let (remote, key) = (&on.target.remote, &record.key); | |
let runs = checks::by(&self.ctx, remote, key, &runners, filter, false).await?; | |
let since = item.queue.as_ref().map_or("", |q| q.created_at.as_str()); |
An empty since makes every earlier failed run count as since queueing. If an item here can lack a queue record, prefer a fallback that matches nothing; otherwise say why it can't happen.
Carried forward: the change this pull makes at ed82c3c7b54a01cadf7ab90fcc0114e9b9e27757 is the one reviewed at d282056e3fd4a888f6c5d43c0612529f0d7f6bdc, so that review stands. What is new comes from its base.
9 passed, 1 skipped
What changes
A failed pre-flight ejects at once. A pre-flight run that failed since the pull was queued now ejects it on whatever tip it ran on. Before, a failure counted only on main's current tip, so when main moved during the run (a
testpre-flight takes about six minutes), the runner ran it again at the same head on the new tip. A failure from before the pull was queued still runs again on a new tip, as main may have fixed it since.A conflict with a pull ahead waits. A pull that conflicts only with pulls stacked ahead of it in its batch stays queued instead of being ejected. If those pulls land, the next batch is built on main with them in it, where the conflict is with main and ejects the pull, saying to merge main in. If they fail, the pull goes on without ever having been ejected.
A queued record names its head.
dev.knotless.queuerecords written asqueuedcarrycommit, the head queued, as ejections already do.Why
From the queue's history (765 queue records and 268 ejections, 10-22 to 11-02): pulls that were ejected took a median of 57 minutes to land, against 13 for those that never were. On a busy main, a deterministic pre-flight failure was run two or three times before ejecting. One pull failed the same test at the same head on three tips, from 17:47 to 18:05, holding the runner and its author for about 18 minutes when 6 would do. Conflicts were the stickiest kind of ejection: 58% of pulls requeued after one were ejected again. Some of those were ejected for conflicting with a pull ahead that then failed.
Without the head on a queued record, the history could not say whether a pull requeued after an ejection had changed.
How it was checked
The pre-flight unit test gains a case for a failure since queueing on an older tip. The merge queue's run test now expects the conflicting pull to wait, then be ejected against main once the pull ahead lands, and checks that a queued pull's head shows in
knotless queue --json. Left to CI.Shortcomings
A pull held for a conflict with one ahead of it learns of the conflict one batch later than before, about five minutes, when that pull lands. The ejection reason now says to merge main in rather than naming the pull ahead.