Mirror sync: negotiate from the default branch and recent tips, not only changed refs' old tips - #51
Merged
Merged
Conversation
…nly changed refs' old tips The sync fetch offered only the old local tips of the changed refs as --negotiation-tip. For a rebased/force-pushed branch (or a batch of a stale branch plus a new one) the old tip's ancestry stops at the old fork point, so the server had to send every commit on the base branch since then - up to the entire recent history of a large repository - and the fetch blew the sync budget. Always offer the mirror's HEAD branch, main/master and (when a commit-graph makes it cheap) the 32 most recently committed branch tips in front of the old tips, deduplicated and still capped at 1000. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
… name guessing Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The post-step mirror sync fetches only the refs that
ls-remoteshows changed, and restricts negotiation with--negotiation-tip(so git does not walk all local refs on a cold disk). Until now the only tips offered were the old local tips of the changed refs;main/masterwas guessed by name and added only when every changed ref was new locally.That is the right common ancestor for a fast-forward push, but not for a rebased / force-pushed branch (or a batch of one stale branch + one new branch): the old tip's ancestry ends at the old fork point, so the server has to ship everything on the base branch since then. On a large monorepo with long-lived stacked branches that is hundreds of MB to GBs, the fetch blows the 120 s sync budget, and the disk is released uncommitted. In production this shows up as a strictly bimodal sync-fetch distribution (tens of thousands of fetches < 5 s, almost nothing in between, then a cluster at the budget) with 1–5 changed refs per timed-out job.
Change: add base tips in front of the old tips (old tips are kept — they are still the best common for fast-forwards), and stop guessing the default branch's name:
diffMirrorRefs(lsRemote, localRefs, hints?: {defaultBranchRef, recentTips})— the cap now lives here, and base tips are ordered first so a >1000-ref batch trims old tips rather than the default branch.mirrorHeadRef()=git symbolic-ref -q HEAD(the upstream default branch as ofclone --mirror). It is deliberately not refreshed from upstream each sync: asking a remote forHEADalone (ls-remote origin HEAD,remote set-head -a) sends noref-prefixin protocol v2 (verified withGIT_TRACE_PACKETon git 2.34), so it pulls the complete advertisement — the very thing the ls-remote-first sync avoids.recentBranchTips()=for-each-ref --sort=-committerdate --count=32 refs/heads, gated onhasCommitGraph()(the same gate used forfetch.writeCommitGraph) because the sort parses every branch tip's commit and is only cheap with the graph. Without a graph the behaviour is today's plus the default branch tip.fetch.negotiationAlgorithm=skippingthe extra tips cost ~33 haves in the first round; the server ACKs the current base immediately and the pack shrinks to the genuinely new commits. Worst case (no base tip is an ancestor of the new ref) is identical to today.Unchanged: which refs are fetched, the sync budget, purge/gc/commit-graph handling, refs/pull.
Tests
[defaultTip, oldFeatureTip]), default branchdevelop+ recent tips ordering/dedupe/invalid-oid filtering, default branch ref missing from the mirror, no hints → no tips, 1200-ref batch keeps the default tip under the 1000 cap.__test__/mirror-sync-negotiation-git.test.ts): source repo with default branchtrunk,featureforked early, mirror cloned, thenfeaturerebased onto 40 newertrunkcommits upstream. Control (fetch --negotiation-tip=<old feature tip>only, i.e. current behaviour) receives >120 objects;syncMirrorFromRemoteofferstrunk's tip (via the HEAD symref), gets it ACKed, and receives < 40 objects (just the rewritten commit).dist/index.jsrebuilt.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Link to Devin session: https://app.devin.ai/sessions/a3b42ef15226438faf3f9cdde9662918
Open in Devin Desktop: https://app.devin.ai/desktop/session/a3b42ef15226438faf3f9cdde9662918?variant=devin
Requested by: @piob-io