Skip to content

Commit 98a84f2

Browse files
justrachclaude
andcommitted
fix: WAL re-queues buffer on write failure instead of dropping entries
flushPending/commit previously freed the write buffer before confirming I/O success. If writeAll failed, entries were permanently lost. Now: - writeAll failure: re-queues the buffer (prepended before any new writes) so the background flusher retries on the next cycle - sync failure: still advances synced_lsn since data is in the file (kernel page cache), preventing the WAL from permanently stalling. Error is logged (flushPending) or returned to caller (commit). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9ccd975 commit 98a84f2

1 file changed

Lines changed: 39 additions & 15 deletions

File tree

src/storage/wal.zig

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -180,22 +180,33 @@ pub const WAL = struct {
180180
self.write_buf = .empty;
181181
self.mu.unlock();
182182

183-
var io_err: ?anyerror = null;
183+
// Write entries to file. If writeAll fails, re-queue the buffer so
184+
// entries are retried on the next flush cycle instead of being lost.
184185
self.file.writeAll(to_write.items) catch |e| {
185-
io_err = e;
186+
self.mu.lock();
187+
// Prepend failed entries before any new writes that arrived.
188+
to_write.appendSlice(self.allocator, self.write_buf.items) catch {};
189+
self.write_buf.deinit(self.allocator);
190+
self.write_buf = to_write;
191+
self.io_err_flag = true;
192+
self.flushing = false;
193+
self.cond.broadcast();
194+
self.mu.unlock();
195+
std.log.err("WAL flushPending write error: {}", .{e});
196+
return;
186197
};
187198
to_write.deinit(self.allocator);
188-
if (io_err == null) self.file.sync() catch |e| {
189-
io_err = e;
199+
200+
// Data is in the file. fsync for durability — if sync fails the data
201+
// is still in the kernel page cache (and the file), so advance
202+
// synced_lsn either way to avoid permanently stalling the WAL.
203+
self.file.sync() catch |e| {
204+
self.io_err_flag = true;
205+
std.log.err("WAL flushPending sync error (data written, not fsynced): {}", .{e});
190206
};
191207

192208
self.mu.lock();
193-
if (io_err) |e| {
194-
self.io_err_flag = true;
195-
std.log.err("WAL flushPending I/O error: {}", .{e});
196-
} else {
197-
self.synced_lsn = target;
198-
}
209+
self.synced_lsn = target;
199210
self.flushing = false;
200211
self.cond.broadcast();
201212
self.mu.unlock();
@@ -288,19 +299,32 @@ pub const WAL = struct {
288299
self.mu.unlock();
289300

290301
// ── I/O outside lock ──────────────────────────────────────────────────
291-
var io_err: ?anyerror = null;
292-
self.file.writeAll(to_write.items) catch |e| { io_err = e; };
302+
// If writeAll fails, re-queue the buffer so entries survive for retry.
303+
self.file.writeAll(to_write.items) catch |e| {
304+
self.mu.lock();
305+
to_write.appendSlice(self.allocator, self.write_buf.items) catch {};
306+
self.write_buf.deinit(self.allocator);
307+
self.write_buf = to_write;
308+
self.flushing = false;
309+
self.cond.broadcast();
310+
self.mu.unlock();
311+
return e;
312+
};
293313
to_write.deinit(self.allocator);
294-
if (io_err == null) self.file.sync() catch |e| { io_err = e; };
314+
315+
// Data written — fsync for durability. If sync fails, still advance
316+
// synced_lsn (data is in page cache) but return the error to caller.
317+
var sync_err: ?anyerror = null;
318+
self.file.sync() catch |e| { sync_err = e; };
295319
// ─────────────────────────────────────────────────────────────────────
296320

297321
self.mu.lock();
298-
if (io_err == null) self.synced_lsn = target;
322+
self.synced_lsn = target;
299323
self.flushing = false;
300324
self.cond.broadcast();
301325
self.mu.unlock();
302326

303-
if (io_err) |e| return e;
327+
if (sync_err) |e| return e;
304328
}
305329

306330
// ── Checkpoint ────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)