Skip to content

feat(git): skip snapshot upload when HEAD is unchanged#395

Merged
worstell merged 1 commit into
mainfrom
worstell/snapshot-unchanged-skip
Jul 21, 2026
Merged

feat(git): skip snapshot upload when HEAD is unchanged#395
worstell merged 1 commit into
mainfrom
worstell/snapshot-unchanged-skip

Conversation

@worstell

@worstell worstell commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

When the last completed generation captured the same mirror HEAD recently enough (snapshot-max-age, default 24h) and the cache entry still exists, snapshot generation skips the tar/zstd/upload entirely. The cached bytes and ETag stay stable, so in-flight snapshot streams and If-Match chunk reads remain valid across generation intervals when nothing changed — overnight ETag collisions drop to ~zero.

The captured commit is recorded on the shared coordination record at completion. A skip releases its claim instead of recording a completion, so the max-age bound keeps tracking the last actual upload and peers aren't suppressed for the full claim TTL. A lost cache entry (crash, expiry, manual delete) fails the existence check and regenerates immediately.

Applies to base, LFS, and mirror snapshots. Mirror snapshots use HEAD as a proxy for the whole mirror: non-HEAD ref drift can be omitted for up to snapshot-max-age, which restored pods cover by fetching immediately on startup.

@worstell
worstell force-pushed the worstell/snapshot-unchanged-skip branch 3 times, most recently from 78ed43f to 8a10581 Compare July 21, 2026 22:08
Base automatically changed from worstell/snapshot-generation-coordination to main July 21, 2026 22:34
@worstell
worstell force-pushed the worstell/snapshot-unchanged-skip branch from 8a10581 to a17628a Compare July 21, 2026 22:37
@worstell
worstell marked this pull request as ready for review July 21, 2026 22:45
@worstell
worstell requested a review from a team as a code owner July 21, 2026 22:45
@worstell
worstell requested review from stuartwdouglas and removed request for a team July 21, 2026 22:45

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a17628a87c

ℹ️ 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".

Comment thread internal/strategy/git/snapshot.go Outdated
Comment thread internal/strategy/git/snapshot_test.go Outdated
When the last completed generation captured the same mirror HEAD recently
enough (snapshot-max-age, default 24h) and the cache entry still exists,
skip tar/zstd/upload so the cached bytes and ETag stay stable. A stable
ETag keeps in-flight snapshot streams and If-Match chunk reads valid
across generation intervals when nothing changed.

The commit is recorded on the shared coordination record at completion.
Skips record a checked-at timestamp instead of a completion: max-age
keeps tracking the last actual upload, while peers stay suppressed for
the interval so an unchanged repo gets one fetch-and-check per interval
rather than one per replica.

Mirror snapshots use HEAD as a proxy for the whole mirror: non-HEAD ref
drift can be omitted for up to snapshot-max-age, which restored pods
cover by fetching immediately.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019f85b8-d346-7243-961f-fbf2236a7f94
@worstell
worstell force-pushed the worstell/snapshot-unchanged-skip branch from a17628a to d50fce0 Compare July 21, 2026 23:03
@worstell
worstell merged commit 118f4ac into main Jul 21, 2026
7 checks passed
@worstell
worstell deleted the worstell/snapshot-unchanged-skip branch July 21, 2026 23:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d50fce039c

ℹ️ 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".

return false
}
rec, ok := c.gens.Get(snapshotGenKey(job, upstreamURL))
return ok && rec.Commit == commit && !rec.CompletedAt.IsZero() && c.now().Sub(rec.CompletedAt) < maxAge

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep max-age valid until the next snapshot check

When a snapshot job runs just before snapshot-max-age elapses, this still returns true; the generator then skips the upload and records CheckedAt, causing Claim to suppress every replica for another full snapshot interval. During that suppression window the cached snapshot can be served older than the configured max age (and can even expire if max age is near the backend TTL), so the max-age bound no longer actually bounds unchanged snapshots. Consider only skipping when the last upload will remain within maxAge until the next check, or cap CheckedAt freshness at CompletedAt + maxAge.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants