fix: support concurrent documentation deploys to GitHub Pages - #98
fix: support concurrent documentation deploys to GitHub Pages#98jamesfredley wants to merge 2 commits into
Conversation
When multiple release/snapshot builds publish documentation at the same time, their pushes to the documentation branch race. Previously a rejected push triggered a plain `git pull --rebase`, which stops on a merge conflict in the shared folders (latest, X.X.x, index.html, snapshot) and, under `set -e`, kills the whole job - so concurrent releases fail. Make the retry loop concurrency-safe: - Rebase the deploy commit onto the remote with `-X theirs` so shared-folder file conflicts auto-resolve in favour of this deploy, and add bounded backoff with jitter between attempts. - Reconcile owned paths after the rebase: purge-owned folders are restored wholesale from the deploy commit (clean last-writer-wins) while merge-mode folders and other deploys' independent version folders are preserved. - Keep `latest` as highest-version-wins: a lower release replayed after a concurrent higher release yields `latest` back to the remote instead of overwriting it with older docs. - Resolve modify/delete rebase conflicts that `-X theirs` cannot handle (a file this deploy purges but a concurrent deploy modified) by driving the rebase to completion deterministically instead of aborting. Add a Testcontainers regression test that lands a concurrent deploy modifying a purged file and asserts the retry recovers cleanly. Assisted-by: claude-code:claude-4.8-opus
There was a problem hiding this comment.
Pull request overview
This PR makes the deploy-github-pages GitHub Action resilient to concurrent documentation deployments by changing the push-retry strategy to rebase onto the remote with deterministic conflict handling, then reconciling “owned” paths to enforce intended last-writer-wins semantics. It also adds a regression test that simulates a real modify/delete conflict caused by a concurrent deploy.
Changes:
- Update the deploy action’s push-retry loop to use
git pull --rebase -X theirs, add bounded backoff, and deterministically resolve modify/delete conflicts during rebase. - Track and reconcile “owned” publish paths after rebase (including special handling so
latestremains highest-version-wins). - Add a Testcontainers-based regression test covering a concurrent deploy that modifies a file this deploy purges.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| deploy-github-pages/entrypoint.sh | Implements concurrency-safe push retries with rebase-based conflict resolution and owned-path reconciliation logic. |
| tests/src/test/groovy/org/apache/grails/github/DeployGithubPagesSpec.groovy | Updates existing retry-log expectation and adds a new regression test for modify/delete conflict recovery. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses review feedback on the concurrent-deploy reconcile logic. The authoritative restore commands in reconcile_owned_paths previously used `|| true`, so if restoring an owned documentation path failed, the action would silently stage and commit a deletion and publish broken/missing docs. - Owned purge/keep paths always exist in the deploy commit, so their `git checkout`/`git add` restores are now unmasked: a failure fails the job under `set -e` instead of committing a silent deletion. Only the best-effort pre-clean `git rm --ignore-unmatch` stays lenient. - The "latest" yield path only restores from the remote when the remote actually publishes it (guarded by `git ls-tree`); otherwise the staged removal stands, avoiding an abort on a now-absent pathspec. The `git ls-tree` check is unmasked so an invalid FETCH_HEAD fails closed. Assisted-by: claude-code:claude-4.8-opus
|
Wouldn't it be better to have a concurrency key or some other solution that blocks the run? |
This can be added per job, so we can have a unique concurrency key per job. I don't believe this change is warranted. |
|
apache/grails-core#15988 is an alternative and I think a better solution than complicating the action. |
|
I'm closing this for now since I've merged the other change. let's revisit if you disagree |
|
I'm still against this solution because we're relying on force repushing. We need to solve this outside of the action. |
|
apache/grails-core#15988 (comment) shows why the original solution didn't work. The reason I'm so against this change is we're effectively trying to solve a shared resource issue inside of a thread instead of outside of it. |
What
Makes the
deploy-github-pagesaction safe when multiple documentation deploys publish concurrently. Today, when several release/snapshot builds push to the documentation branch at the same time, their pushes race: a rejected push runs a plaingit pull --rebase, which stops on a merge conflict in the shared folders and - underset -e- kills the whole job. Concurrent releases therefore fail (observed inapache/grails-coredoc publishing).Why it happens
Different releases never collide on their specific
X.X.Xfolders, but they all write the shared paths:latest, the genericX.X.x, the rootindex.html, and thesnapshotfolder. A concurrent deploy that lands first advances the branch, our push is rejected, and the naive rebase conflicts on those shared paths and aborts.The fix
In
deploy-github-pages/entrypoint.sh, the push-retry loop is now concurrency-safe:-X theirs+ backoff. The deploy commit is replayed onto the remote so a concurrent deploy's independent version folders are preserved, and shared-folder file-content conflicts auto-resolve in favour of this deploy. Retries use bounded backoff with jitter.PURGE_EXISTING=true) are restored wholesale from this deploy's commit (clean last-writer-wins), so a shared folder never becomes a union of both deploys' files. Merge-mode folders (PURGE_EXISTING=false) are deliberately left to the-X theirsmerge so a concurrent deploy's independent changes survive.latestis highest-version-wins. A lower release replayed after a concurrent higher release yieldslatestback to the remote instead of overwriting it with older docs.-X theirscannot auto-resolve a modify/delete conflict (a file this deploy purges that a concurrent deploy modified). Instead of aborting and re-hitting the same conflict until attempts are exhausted, the rebase is now driven to completion deterministically.Tests
Adds a Testcontainers regression test to
DeployGithubPagesSpecthat lands a concurrent deploy modifying a file this deploy purges (a real modify/delete conflict) and asserts the retry recovers cleanly - the deploy's own content wins and the stale file is removed. All existing deploy tests pass.Note / follow-up
apache/grails-static-websiteexhibits the same class of race, but in a separate Gradle publish task (:publishMainSitepushing toapache/grails-websiteasf-site-production), which lives outside this repository. That will need a separate follow-up fix in that project - it is not addressed here.