Skip to content

fix(wal): read the log and replay refs through the caller's manifest - #12

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-wal-read-log-through-manifest
Oct 23, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/fix-wal-read-log-through-manifest

Conversation

@yordis

@yordis yordis commented Oct 23, 2026

Copy link
Copy Markdown
Member

@cursor

cursor Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches core WAL read/replay and event publishing semantics; behavior change is intentional for concurrent checkpointing but any caller still assuming implicit freshness in read_log would be wrong.

Overview
WAL log reads and ref replay are tied to an explicit manifest instead of always re-fetching the newest store manifest inside read_log. Callers pass the manifest their seq bounds came from (or call new fresh_manifest() when they want the latest). Checkpoint/ref-replay helpers (refs_as_of, refs_at_seq, checkpoint_times, etc.) follow the same pattern.

This fixes the events bridge skipping a committed seq when catch-up uses a manifest with head_seq = N but a concurrent checkpoint on another instance advances the store manifest and trims segments before read_log runs. The bridge now reads (cursor, head] through the manifest it already synced; CLI, settings/history, policy dry-run, overview UI, and materialize paths are updated accordingly.

Adds an integration test for checkpoint during bridge catch-up (sibling folds seq 2 while the bridge holds an older manifest view with freshness_ttl).

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

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ 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: 7b85225f-9d90-4614-80e3-b6fde7bd857f
📥 Commits

Reviewing files that changed from the base of the PR and between 7c0a6c9 and 600e10b.

📒 Files selected for processing: 14
  • crates/walgit-cli/src/settings_cmd.rs
  • crates/walgit-cli/src/wal_cmd.rs
  • crates/walgit-server/src/bridge.rs
  • crates/walgit-server/src/settings.rs
  • crates/walgit-server/src/web/ui.rs
  • crates/walgit-server/tests/events.rs
  • crates/walgit-server/tests/follow.rs
  • crates/walgit-server/tests/harness.rs
  • crates/walgit-server/tests/sim.rs
  • crates/walgit-wal/src/handle.rs
  • crates/walgit-wal/src/log_reader.rs
  • crates/walgit-wal/src/snapshots.rs
  • crates/walgit-wal/src/sync.rs
  • crates/walgit-wal/tests/wal.rs
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Good code is its own best documentation. As you're about to add a comment, ask yourself, 'How can I improve the code so that this comment isn't needed?' - Steve McConnell |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • 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.

catch_up took head_seq from one manifest while read_log resolved a fresher
one; a checkpoint in between trims that one's segments, so the bridge
emitted nothing and advanced its cursor past the push. read_log,
refs_at_seq, refs_as_of and the checkpoint-time helpers now take the
caller's manifest, and fresh_manifest() is the explicit way to get the
newest one.

Round trips: the bridge catch-up, the settings history and policy dry-run,
the repository overview and `walgit settings history` each made two
conditional manifest GETs (the sync, then read_log's own) and now make one;
`walgit wal ls` and `wal show` still make one. No budget in
docs/ROUNDTRIPS.md changes.

(cherry picked from commit 7844344)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis force-pushed the yordis/fix-wal-read-log-through-manifest branch from 3b1ef63 to 600e10b Compare October 23, 2026 07:24
@yordis
yordis force-pushed the yordis/fix-wal-keep-412-log-segment branch from fdc6d25 to 5afc103 Compare October 23, 2026 07:24
Base automatically changed from yordis/fix-wal-keep-412-log-segment to main October 23, 2026 08:19
@yordis
yordis marked this pull request as ready for review October 23, 2026 08:20
@yordis
yordis merged commit ac93a0d into main Oct 23, 2026
6 of 11 checks passed
@yordis
yordis deleted the yordis/fix-wal-read-log-through-manifest branch October 23, 2026 08:21
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