Skip to content

fix: ensure durable writes actually wait for WAL flush#6368

Merged
westonpace merged 2 commits intolance-format:mainfrom
hamersaw:bug/wal-durable-writes
Apr 2, 2026
Merged

fix: ensure durable writes actually wait for WAL flush#6368
westonpace merged 2 commits intolance-format:mainfrom
hamersaw:bug/wal-durable-writes

Conversation

@hamersaw
Copy link
Copy Markdown
Contributor

@hamersaw hamersaw commented Apr 1, 2026

track_batch_for_wal was returning a pre-resolved watcher instead of the actual BatchDurableWatcher, so durable_write=true never blocked waiting for the WAL flush to complete. Also include close_duration in benchmark timing for accurate end-to-end throughput reporting.

track_batch_for_wal was returning a pre-resolved watcher instead of
the actual BatchDurableWatcher, so durable_write=true never blocked
waiting for the WAL flush to complete. Also include close_duration
in benchmark timing for accurate end-to-end throughput reporting.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added the bug Something isn't working label Apr 1, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/dataset/mem_wal/write.rs 75.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

The change seems fine but it raises a question. Why wasn't this caught by a failing unit test? Do we need unit tests to ensure that the durable write feature is actually working as intended?

Verifies that track_batch returns a watcher wired to the real WAL
watermark that blocks until flush completes, not a pre-resolved one.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hamersaw
Copy link
Copy Markdown
Contributor Author

hamersaw commented Apr 2, 2026

The change seems fine but it raises a question. Why wasn't this caught by a failing unit test? Do we need unit tests to ensure that the durable write feature is actually working as intended?

I just don't think there was a test. I just noticed it because I turned on durable writes and they were completing in like single ms. I just added a unit test that should catch this, but timing things are tough sometimes.

@westonpace westonpace merged commit 2b62004 into lance-format:main Apr 2, 2026
28 checks passed
@hamersaw hamersaw deleted the bug/wal-durable-writes branch April 2, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants