Skip to content

fix(wal): drain the post-publish commit-graph update when a handle retires - #55

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-wal-commit-graph-drain
Oct 29, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/fix-wal-commit-graph-drain

Conversation

@yordis

@yordis yordis commented Oct 29, 2026

Copy link
Copy Markdown
Member
  • A reopen that resets the local copy could race the commit-graph task a push left running; that task wrote into the directory being removed, so the reset failed with DirectoryNotEmpty and the create/open tests flaked on CI.

…tires

A reopen that resets the local copy could race the commit-graph task a push left running, which wrote into the directory being removed and failed the reset with DirectoryNotEmpty.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 44440b96-5cab-4343-8fa2-8e1177b318c2
Commits

Reviewing files that changed from the base of the PR and between fa1dd1a and 7d2edfa.

Files selected for processing: 2
  • crates/walgit-wal/src/publish.rs
  • crates/walgit-wal/tests/wal.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes locking and liveness on the async post-publish path; behavior is aligned with existing pack/sync guards but could affect timing of commit-graph updates under contention.

Overview
Fixes a race where the background commit-graph task spawned after a successful push could keep writing into the local repo directory while open/create was resetting or wiping that same directory, which surfaced as DirectoryNotEmpty and flaky create/open tests on CI.

The post-publish task now takes pack_mutex (via the same timed lock helper used elsewhere) before maintain_commit_graph, and exits immediately if ensure_live() fails so retired handles do not touch disk after a reopen or recreate.

Integration tests that simulate restart by dropping the registry and opening a new one use a new restart_config helper that sets git.commit_graph = false, because an in-process “restart” does not stop the old background task the way a real process restart would.

Reviewed by Cursor Bugbot for commit 7d2edfa. Bugbot is set up for automated code reviews on this repo. Configure here.

@yordis
yordis merged commit b2ca34d into main Oct 29, 2026
7 checks passed
@yordis
yordis deleted the yordis/fix-wal-commit-graph-drain branch October 29, 2026 07:18
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.

1 participant