chadtmiller.com/knotless
git that lives in your Atmosphere account
A push no longer waits on the appview after it lands
MergedThis moves the appview reads for a branch push off the path after it lands. The members question starts when git lists the refs. The pulls question starts, on that connection, as the push goes out. forge::landed then only awaits the result.
I read every changed file and found nothing that must change. The points I checked:
- The
Watchis aborted on drop, so a push that errors or is refused leaves no task running. - An early answer for an owner other than the current remote's is discarded, so the transfer case asks again.
- The error path is as before: a failed read is said and left.
- Moving the read ahead of the push has the stated effect: a pull opened mid-push is not marked merged. The docs and the change file say so.
- The docs page and change file are present, as AGENTS.md asks.
I did not check that the 100% coverage gate covers the new Drop and the None and transfer branches; CI's test will show that. The numbers the body promises are not in yet, so check them before queueing.
Since my approval at f5d1575, only the README and the performance docs changed. They now carry the measured numbers: 299 ms for a push with an appview against 507 ms before, which is close to the 296 ms push without one. That was the open item in my last review. The text matches the design: the appview reads go out beside the push, and git still waits for the answers after the push lands. The old 507 ms figure is kept as the cost of asking after the push. The code is unchanged, so my earlier approval stands. I did not re-run the bench; the figures are the author's.
8 passed, 3 skipped
A
git pushof a branch, from a client with an appview, waited for the appview after the push had landed: four round trips over https, as long again as the push itself. It now asks while the push goes out, so git hears of the push when the PDS answers. The part to look at is what is read before the push lands, inforge::watch.Claude Fable 5.2
What changes
When the pulls are read. After a push moved a branch,
git-remote-knotlessopened a connection to the appview and asked it two things in turn, who may push to the owner's repositories and what is said about this one, to mark merged the pulls the push landed. The first is now asked as git lists the refs to push (Knotless::ask_early), and the second as the push goes out (forge::watch), on the connection the first opened.forge::landedtakes what that read and no longer reads anything itself until it has a pull to mark.What a push sees. The pulls are as they were when the push went out, not when it landed: one opened in between is not marked merged by this push. A push that moves no branch, because it was refused or pushed only tags, drops the read and waits for nothing. A repository found in another account than the one git named (a transfer) asks again, of its owner now.
The appview hears of more pushes. Every
git pushto a public repository asks the first question, where only one that moved a branch did: a push with nothing to push, or of tags, now asks it and uses none of the answer.Target::read_withtakes the members already asked for;org::public_writersis the questionorg::fillasked for a public repository, by itself.Why
Measured in The bench reaches the PDS over https, and counts connecting: at 50 ms of round-trip time a push took 296 ms, and 507 ms from a client with an appview, which is every installed one. The four round trips were two to connect and one for each question, all after the PDS had taken the push.
Two smaller changes were weighed. Asking both questions at once after the push saves one round trip of the four. Asking both as the push goes out, with nothing asked earlier, saves two: the connection still takes longer than the push's one request. Only a connection begun before the refs are read is open in time, and the first question needs no more than the owner's DID, which the helper has from its start.
Leaving the marking to a process git does not wait for, as compaction is left, would cost nothing in round trips either, but
knotless: <pull> merged, as main now has its headwould no longer reach whoever pushed.How it was checked
The bench's row for a push of a branch from a client with an appview, release build on an M1 Pro, p50, over https:
With
--plain, which leaves the requests git waits for: 4.0 before, 2.0 after.docs/reference/performance.mdand the README have the new figures.pushes_mark_the_pulls_they_land_mergedstill holds what a push marks, the transfer test pushes through a URL that leads to another account, and it now also pushes to the archived repository, which is refused, and must say nothing of pulls. All eight checks passed at the first commit,testwith its coverage gate among them;cargo clippy -p knotless --lib -- -D warningsandnode harness/docs.mjspass here.Shortcomings