Skip to content

fix(wal): make TruncateAll early return work when AllowEmpty is true#3124

Merged
wen-coding merged 1 commit into
mainfrom
wen/fix_truncateall_early_return
Mar 26, 2026
Merged

fix(wal): make TruncateAll early return work when AllowEmpty is true#3124
wen-coding merged 1 commit into
mainfrom
wen/fix_truncateall_early_return

Conversation

@wen-coding

@wen-coding wen-coding commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The previous TruncateAll early return (first == 0 && last == 0) only matched the AllowEmpty=false never-written case. With AllowEmpty=true, an empty WAL reports first > last, so the early return never fired and TruncateAll sent a pointless truncate request to the underlying library.
  • Change to first > last which correctly covers empty states when AllowEmpty is true, currently we don't call this when AllowEmpty is false.
  • Remove misleading doc comment that implied TruncateAll checks AllowEmpty itself — the check is in the underlying tidwall/wal.

Test plan

  • TestTruncateAllOnEmptyLog already covers calling TruncateAll on an empty log with AllowEmpty=true — now hits the early return instead of round-tripping through sendTruncate
  • All existing WAL tests pass

@github-actions

github-actions Bot commented Mar 26, 2026

Copy link
Copy Markdown

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMar 26, 2026, 4:52 PM

The previous check (first == 0 && last == 0) only matched the
AllowEmpty=false never-written case. With AllowEmpty=true, an empty
WAL reports first > last, so the early return never fired and
TruncateAll sent a pointless truncate request to the library.

Change to first > last which correctly covers all empty states.
Also remove misleading doc comment that implied TruncateAll checks
AllowEmpty itself — the check is in the underlying tidwall/wal.

Made-with: Cursor
@codecov

codecov Bot commented Mar 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.58%. Comparing base (78c53dd) to head (eec4f8a).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3124   +/-   ##
=======================================
  Coverage   58.58%   58.58%           
=======================================
  Files        2099     2099           
  Lines      173525   173525           
=======================================
+ Hits       101657   101659    +2     
+ Misses      62805    62804    -1     
+ Partials     9063     9062    -1     
Flag Coverage Δ
sei-chain-pr 70.44% <100.00%> (?)
sei-db 70.41% <ø> (ø)

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

Files with missing lines Coverage Δ
sei-db/wal/wal.go 71.28% <100.00%> (+0.66%) ⬆️
🚀 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.

@wen-coding wen-coding changed the title fix(wal): TruncateAll early return covers AllowEmpty empty case fix(wal): make TruncateAll early return work when AllowEmpty is true Mar 26, 2026
@wen-coding wen-coding force-pushed the wen/fix_truncateall_early_return branch from e53b3d3 to eec4f8a Compare March 26, 2026 16:51
@wen-coding wen-coding added this pull request to the merge queue Mar 26, 2026
Merged via the queue into main with commit cf2abd8 Mar 26, 2026
39 checks passed
@wen-coding wen-coding deleted the wen/fix_truncateall_early_return branch March 26, 2026 20:56
jewei1997 pushed a commit that referenced this pull request Mar 30, 2026
…3124)

## Summary

- The previous `TruncateAll` early return (`first == 0 && last == 0`)
only matched the `AllowEmpty=false` never-written case. With
`AllowEmpty=true`, an empty WAL reports `first > last`, so the early
return never fired and `TruncateAll` sent a pointless truncate request
to the underlying library.
- Change to `first > last` which correctly covers empty states when
AllowEmpty is true, currently we don't call this when AllowEmpty is
false.
- Remove misleading doc comment that implied `TruncateAll` checks
`AllowEmpty` itself — the check is in the underlying `tidwall/wal`.

## Test plan

- `TestTruncateAllOnEmptyLog` already covers calling `TruncateAll` on an
empty log with `AllowEmpty=true` — now hits the early return instead of
round-tripping through `sendTruncate`
- All existing WAL tests pass
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.

3 participants