Skip to content

Conversation

@lunny
Copy link
Member

@lunny lunny commented Jan 30, 2026

Fix #36448

Removed unnecessary parameters from the LFS GC process and switched to an ORDER BY id ASC strategy with a last-ID cursor to avoid missing or duplicating meta object IDs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 30, 2026
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Jan 30, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes the LFS GC hang reported in #36448 by making LFS meta-object iteration stable and monotonic (ID-based cursor), preventing repeated batches when records are updated during iteration.

Changes:

  • Simplified IterateLFSMetaObjectsForRepoOptions by removing unused/unsafe iteration modes.
  • Reworked IterateLFSMetaObjectsForRepo to page via ORDER BY id ASC + id > lastID cursor (no offset paging).
  • Added regression tests covering GC autofix and iteration behavior when rows are updated mid-iteration.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
services/repository/lfs.go Stops passing the removed iteration parameters; relies on the new ID-cursor iteration behavior.
models/git/lfs.go Implements ID-based cursor pagination for LFSMetaObject iteration and removes the old ordering/offset options.
services/repository/lfs_test.go Adds a repo-scoped GC autofix test (and imports modules/test for mocking).
models/git/lfs_test.go Adds a regression test ensuring iteration doesn’t skip/hang when meta rows are updated during the loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/v1.25 lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/go Pull requests that update Go code type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hang when running the gc-lfs doctor

4 participants