Skip to content

fix(checkpoint): preserve newer same-ref queue updates - #2550

Open
Vishal202-rgb wants to merge 1 commit into
entireio:mainfrom
Vishal202-rgb:fix/2008-preserve-same-ref-queue
Open

Vishal202-rgb wants to merge 1 commit into
entireio:mainfrom
Vishal202-rgb:fix/2008-preserve-same-ref-queue

Conversation

@Vishal202-rgb

Copy link
Copy Markdown

Summary

Fixes #2008.

  • Preserve a newer same-ref queue entry added while a checkpoint push is in flight.
  • Keep readLocked de-duplication behavior for Drain and Peek.
  • Add a regression test covering the same-ref enqueue race.

Testing

  • git diff --check
  • Added TestPushQueue_RemovePreservesNewerSameRefEntry

Note: The Go test suite was not run locally because Go is not installed in the environment.

@Vishal202-rgb
Vishal202-rgb requested a review from a team as a code owner September 21, 2026 17:49
Copilot AI lite review requested due to automatic review settings September 21, 2026 17:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Concurrent drains can still delete a newer same-reference queue entry; documentation issues also remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Low severity

Open (3)
What changed in this PR

Updates checkpoint push-queue handling to preserve newer same-reference entries during in-flight pushes.

Changes:

  • Separates raw queue parsing from de-duplicated reads.
  • Removes drained queue occurrences.
  • Adds same-reference enqueue regression coverage.
File Summary
cmd/​entire/​cli/​checkpoint/​pushqueue.go Queue handling changes; concurrent drains can still remove newer entries. Documentation nits remain.
cmd/​entire/​cli/​checkpoint/​pushqueue_test.go Adds regression coverage for newer same-reference entries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +179 to +180
if count := remaining[key]; count > 0 {
remaining[key] = count - 1
// de-duplicated set and is worth compacting: rawLines > len(refs) exactly when
// there were redundant lines.
func (q *PushQueue) readLocked(root *os.Root) (refs []plumbing.ReferenceName, rawLines int, err error) {
func (q *PushQueue) readAllLocked(root *os.Root) (refs []plumbing.ReferenceName, rawLines int, err error) {

// readLocked parses the queue file into de-duplicated refs, preserving first-seen
// order. The caller must hold the lock. Malformed lines are skipped rather than
// failing the whole drain ? a single bad record must not strand every queued ref.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

git-refs push queue can drop a newer same-ref update during an in-flight push

2 participants