Skip to content

Commit ac9c733

Browse files
authored
server/replication: fsync frame data in wallog before writing header (#2200)
Currently in logger.rs we are not issuing any fsyncs at all. We do `flush()` in `write_header()` but this is a no-op on Unix because there are no userspace buffers to flush since `write_all_at()` writes directly to kernel. Not fsyncing can cause the following bug: - We write X frames - We write wallog header (containing frame count X) - Kernel reorders writes so that header has been written out but not all of the frame data - Crash - Sqld starts up, reads wallog header which claims X frames exist in wallog, but only Y frames (Y < X) have been written - short read in read_frame_byte_offset_mut() returns an error and crashes the server It's also important to sync after writing frames but before writing the header so that there is no write reordering scenario where the header is persistent before the frame data is.
2 parents 2d50cd0 + 214baf1 commit ac9c733

File tree

1 file changed

+6
-1
lines changed
  • libsql-server/src/replication/primary

1 file changed

+6
-1
lines changed

libsql-server/src/replication/primary/logger.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,10 @@ impl LogFile {
177177
}
178178

179179
pub fn commit(&mut self) -> anyhow::Result<()> {
180+
// Ensure frame data is durable before updating the header.
181+
// Without this, a crash could leave the header claiming more frames
182+
// than actually exist on disk.
183+
self.file.sync_data()?;
180184
self.header.frame_count += self.uncommitted_frame_count.into();
181185
self.uncommitted_frame_count = 0;
182186
self.commited_checksum = self.uncommitted_checksum;
@@ -193,6 +197,7 @@ impl LogFile {
193197
pub fn write_header(&mut self) -> anyhow::Result<()> {
194198
self.file.write_all_at(self.header.as_bytes(), 0)?;
195199
self.file.flush()?;
200+
self.file.sync_all()?;
196201

197202
Ok(())
198203
}
@@ -387,7 +392,7 @@ impl LogFile {
387392
..self.header
388393
};
389394
new_log_file.header = new_header;
390-
new_log_file.write_header().unwrap();
395+
new_log_file.write_header()?;
391396
// swap old and new snapshot
392397
// FIXME(marin): the dest path never changes, store it somewhere.
393398
atomic_rename(&to_compact_log_path, path.join("wallog")).unwrap();

0 commit comments

Comments
 (0)