Skip to content

Open read-only changelog WALs from a private view of the directory - #4441

Open
masih wants to merge 4 commits into
mainfrom
masih/1791138739-readonly-wal-snapshot
Open

masih wants to merge 4 commits into
mainfrom
masih/1791138739-readonly-wal-snapshot

Conversation

@masih

@masih masih commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

Every read-only open of a memIAVL changelog goes through tidwall's wal.Open on the live directory: the seidb evm-logical-digest sidecar scan, historical ABCI proof queries and CacheMultiStoreForExport via CommitStore.LoadVersion(v, true), and the changelog-range reader. On load, tidwall finishes any interrupted TruncateFront it finds, deleting the segments ahead of a .START marker and renaming the marker, and the repair path may os.Truncate a torn tail in place. When the open lands between a live writer's creation of .START and its removal of the old segments, the reader deletes those segments first. The writer's own remove then fails with ENOENT, tidwall marks the log corrupt, and handleTruncate reports it as a fatal WAL error. NoRepairOnOpen never covered this cleanup.

wal.Config.ReadOnly now opens the log from a private view created beside the directory (<dir>-readonly-*-tmp). The view hard-links the segments and copies the files the open may rewrite in place: the tail segment, any .START/.END marker, and the last file with a segment-length name, which the tail repair truncates and which differs from the tail when a stray file sorts after it. Cleanup and repair therefore touch only the view. If the writer removes a segment while the view is being built, the build is retried. A read-only WAL refuses Write and truncations, never prunes, and removes the view on Close. memiavl.OpenDB passes Options.ReadOnly through, and the changelog-range reader sets it. I rejected skipping tidwall's open, since that would mean reimplementing its segment loading.

There is no consensus or state impact, and writers are unchanged. While a read-only log is open, its view keeps segments the writer has truncated alive on disk, and each open copies the tail segment (up to 20 MB). The -tmp suffix means memIAVL's startup removeTmpDirs deletes views left behind by a crashed reader, and OpenDB closes the WAL, removing its view, when it fails after opening it. TestReadOnlyOpenDuringWriterTruncation runs four read-only openers against a writer that keeps truncating. Before this change the same loop made the writer fail within a few writes with remove ...: no such file or directory; with it, 20 race-enabled runs pass. The wal, memiavl, composite, seidb operations and rootmulti packages pass under -race.

A read-only open went through tidwall's wal.Open on the live directory, which completes an interrupted TruncateFront and may truncate a torn tail. Racing a live writer's TruncateFront, it removed the writer's segments and made the writer's own truncation fail as a fatal WAL error.

Config.ReadOnly now opens the log from a private view beside the directory, hard-linking its segments and copying the files the open may rewrite, and refuses writes and truncations. memiavl's read-only open and the changelog range reader set it.
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedOct 5, 2026, 8:23 AM

@codecov

codecov Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.69565% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.77%. Comparing base (82c0cda) to head (9dd666b).

Files with missing lines Patch % Lines
sei-db/wal/readonly.go 85.29% 10 Missing ⚠️
sei-db/wal/wal.go 78.94% 4 Missing ⚠️
sei-db/state_db/sc/memiavl/db.go 75.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4441      +/-   ##
==========================================
+ Coverage   56.75%   56.77%   +0.02%     
==========================================
  Files        2126     2127       +1     
  Lines      166836   166925      +89     
==========================================
+ Hits        94691    94775      +84     
- Misses      72140    72145       +5     
  Partials        5        5              
Flag Coverage Δ
sei-chain 54.95% <83.90%> (+0.01%) ⬆️
sei-db 74.81% <ø> (ø)
sei-db-state-db 78.86% <80.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/state_db/sc/memiavl/changelog_range.go 85.39% <100.00%> (ø)
sei-db/state_db/sc/memiavl/opts.go 100.00% <ø> (ø)
sei-db/state_db/sc/memiavl/db.go 80.54% <75.00%> (+0.39%) ⬆️
sei-db/wal/wal.go 76.55% <78.94%> (+0.72%) ⬆️
sei-db/wal/readonly.go 85.29% <85.29%> (ø)

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@masih
masih marked this pull request as ready for review October 4, 2026 18:51
@cursor

cursor Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes WAL open semantics and filesystem layout for read-only consumers alongside live writers; writer paths are unchanged but incorrect view/copy logic could affect concurrent changelog access or disk use.

Overview
Adds wal.Config.ReadOnly, which opens the changelog from a private temp view next to the live directory (hard-linked segments, copied tail/markers that tidwall may repair) so read-only opens no longer run repair or truncate cleanup on the writer’s files.

memiavl passes Options.ReadOnly into changelog open (OpenDB, tree changelog replay), closes the WAL on failed open, and tightens option docs. Read-only WALs reject writes/truncations, skip pruning, and remove the view on Close. New tests cover non-mutation, corruption/stray-file cases, and concurrent writer truncation.

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

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 212a92c. Configure here.

Comment thread sei-db/state_db/sc/memiavl/db.go
@masih

masih commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

@seidroid review

@seidroid seidroid 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.

Read-only changelog WAL opens now run against a private directory beside the log, made of hard links plus copies of the tail and any truncation markers, so tidwall's interrupted-truncation cleanup and torn-tail repair no longer touch a log another writer is using; memiavl read-only opens and the changelog-range reader use it. The approach holds up against the writer races checked, so this is an approval; the remaining items concern how robust the view is and leaks on error paths, and none is a correctness break today. Both of codex's findings are kept: the tail-selection one is downgraded from high to a suggestion because no file that sorts after the tail can trigger the harmful case today, and the trailing-slash one is a nit because every current caller passes a cleaned path.

Non-blocking

3 findings on the changed lines, as inline comments.

  • Cleanup of crashed readers' views depends on memiavl's writer-side removeTmpDirs. That function deletes every *-tmp directory under the memiavl dir at writer startup, including views a live reader in another process still holds open, such as the seidb evm-logical-digest sidecar. tidwall loads segments by path on demand, so a node restart during a sidecar scan now makes the scan fail with ENOENT, which reading the live directory did not do. Consider giving reader views a distinct marker, or a liveness check such as an flock, before removing them.
1 nit, not posted on the code
  • sei-db/wal/readonly.go:26 — (From codex, verified.) With a trailing slash, filepath.Dir(dir) returns the WAL directory itself, so the view would be created inside the log it promises not to modify. Current callers pass filepath.Join output, so this is latent; filepath.Clean(dir) first closes it.

seidroid review · decision approve · session f64ef74857fb4bef9d4980d311f8efcf · turn resp_claude_3e73e9d130f12efd1e2659e91d2f0639 · item d724d0f0b2f45e0994ba913998ddc7db

Findings: 0 blocking | 4 non-blocking | 3 posted inline

Comment thread sei-db/wal/readonly.go Outdated
Comment thread sei-db/state_db/sc/memiavl/db.go
Comment thread sei-db/wal/readonly_test.go
masih added 2 commits October 4, 2026 19:27
…B errors

The view linked the tail segment whenever a file that is not a segment sorted after it, so repairing a corrupted tail truncated the live segment. Copy both the last segment and the file the tail repair targets, close the WAL when OpenDB fails after opening it, clean the source path, and count failed opens in the concurrency test.
@masih

masih commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

@seidroid review

@seidroid seidroid 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.

Since the last review, the read-only view also copies the last segment-length file that the tail repair truncates, OpenDB closes the WAL (and its view) when it fails after opening it, the concurrent test now counts reader failures, and the directory is cleaned before its parent is taken; all three of my earlier threads are resolved. Nothing blocks; codex's new .-directory finding holds but is a nit because callers always pass a path ending in changelog, and my earlier note about removeTmpDirs deleting views that live readers still hold has not been addressed.

Non-blocking

  • Cleanup of crashed readers' views still relies on memiavl's writer-side removeTmpDirs. At writer startup it deletes every *-tmp directory, including views that a live reader in another process, such as the seidb evm-logical-digest sidecar, still holds open. tidwall loads segments by path on demand, so restarting a node during a sidecar scan now makes the scan fail with ENOENT. Before this change, reading the live directory did not fail that way. Consider a distinct marker or a liveness check, such as an flock, before removing a view.
1 nit, not posted on the code
  • sei-db/wal/readonly.go:28 — (From codex, verified.) For dir == ".", filepath.Dir returns ., so the view would be created inside the directory it promises not to modify. Every current caller passes filepath.Join(..., "changelog"), so this cannot happen today; using filepath.Abs instead of filepath.Clean closes it.

seidroid review · decision approve · session f64ef74857fb4bef9d4980d311f8efcf · turn resp_claude_f6bcd0d7b5cb1fb400284a6fe9cc9934 · item 51287ec447845756b75a97c55175e14d

Findings: 0 blocking | 1 non-blocking | 0 posted inline

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant