fix: Restore local mirror warm-hit performance - #4171
Conversation
Co-authored-by: Paul Annesley <pda@users.noreply.github.com>
Co-authored-by: Paul Annesley <pda@users.noreply.github.com>
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6670d4f6fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hasGitCommit(ctx, e.shell, mirrorDir, e.Commit) { | ||
| // Avoid the global reachability scan when remote mirroring is not configured. | ||
| e.shell.Commentf("Commit %q exists in mirror", e.Commit) | ||
| return e.snapshotMirror(ctx, repository, mirrorDir) |
There was a problem hiding this comment.
When a renamed repository or two canonical URLs normalize to the same dirForRepository path and the requested commit already exists in the shared mirror, this return runs before updateRemoteURL, leaving remote.origin.url pointed at the previous repository. This bypasses the collision repair immediately below and exposes a stale-origin mirror to subsequent warm hits and consumers of BUILDKITE_REPO_MIRROR; perform the inexpensive origin repair before taking this fast path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is valid, but intentionally deferred for the emergency hotfix. The no-URL path restores the pre-remote-mirror warm-hit behavior so large mirrors avoid the multi-minute global reachability scans. The stale-origin case requires a repository rename or dirForRepository collision while the requested commit is already present; a later miss reconciles origin before fetching. We’ll follow up by moving the inexpensive origin reconciliation ahead of this return without restoring the reachability scan.
2 participants
Description
Warm on-host Git mirrors regressed after remote-mirror safety checks added global ref-reachability scans to every cached commit lookup. On large mirrors those scans can delay checkout by several minutes even when no remote mirror URL is configured.
Restore the previous presence-only warm-hit path when remote mirroring is skipped specifically because no URL is configured. Remote-mirror-enabled behavior remains unchanged and can be optimized separately.
Context
Production build showing the checkout delay
Changes
for-each-ref.Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go tool gofumpt -extra -w .)Focused mirror-update regressions pass with the race detector. The broad
internal/jobpackage run was intentionally stopped because this urgent hotfix is covered by the focused tests and that package routinely takes 6–7½ minutes locally.Disclosures / Credits
Cursor Agent diagnosed the production regression, implemented the hotfix and regression coverage, and ran independent ship-risk, maintainability, and simplicity reviews.