crates/clawhdf5-agent/src/wal.rs

@@ -13,6 +13,12 @@ use crate::MemoryError;
 
 const WAL_MAGIC: [u8; 4] = [0x45, 0x48, 0x57, 0x4C]; // "EHWL"
 
+/// Bytes before the first entry: [`WAL_MAGIC`] (4) + version (1) + entry
+/// count (4). Named so the offset arithmetic in `open()` — which decides
+/// where an append lands, and therefore whether it is replayable — reads as
+/// a header length rather than a bare 9.
+const WAL_HEADER_LEN: u64 = WAL_MAGIC.len() as u64 + 1 + 4;
+
 /// Current WAL format version: every entry's CRC32 trailer is computed over
 /// its own bytes *chained with the previous entry's stored CRC*
 /// (`crc32(entry_bytes ++ prev_crc.to_le_bytes())`, seeded with 0 for the
@@ -141,13 +147,43 @@ impl WalFile {
                     // be stale from deferred group-commit sync, same
                     // tolerance `read_entries` already has, so the scanned
                     // count is also the more accurate of the two).
-                    let (entries, running_crc) = read_chained_entries(&mut f, 0);
+                    let (entries, running_crc, verified_bytes) = read_chained_entries(&mut f, 0);
                     let entry_count = if entries.is_empty() {
                         header_count
                     } else {
                         entries.len() as u32
                     };
-                    f.seek(SeekFrom::End(0))?;
+                    // Position the append at the end of the VERIFIED prefix,
+                    // and drop anything after it.
+                    //
+                    // This used to `seek(End(0))`, which appends PAST a torn
+                    // tail — the ordinary outcome of a crash mid-append. The
+                    // new entry is then chained to the last good entry, but
+                    // sits on disk behind the garbage:
+                    //
+                    //     [1..N verified][torn bytes][N+1 chained to N]
+                    //
+                    // Replay stops at the torn bytes, so N+1 is unreachable
+                    // FOREVER even though its `append` returned Ok and synced.
+                    // That is silent data loss in the one situation a WAL
+                    // exists for. Truncating to the verified end is the
+                    // standard recovery: the torn tail was never acknowledged
+                    // to any caller, so discarding it loses nothing, and the
+                    // chain then continues from a byte offset that matches
+                    // `running_crc`.
+                    let verified_end = WAL_HEADER_LEN + verified_bytes;
+                    let file_len = f.metadata()?.len();
+                    if file_len > verified_end {
+                        eprintln!(
+                            "clawhdf5-agent: WAL {} has {} unverifiable byte(s) after entry {}; \
+                             discarding them so appends stay replayable",
+                            path.display(),
+                            file_len - verified_end,
+                            entries.len()
+                        );
+                        f.set_len(verified_end)?;
+                    }
+                    f.seek(SeekFrom::Start(verified_end))?;
                     Ok(Self {
                         path: path.to_path_buf(),
                         file: Some(f),
@@ -307,7 +343,7 @@ impl WalFile {
 
         match header[4] {
             WAL_VERSION => {
-                let (entries, _final_crc) = read_chained_entries(&mut f, 0);
+                let (entries, _final_crc, _verified_bytes) = read_chained_entries(&mut f, 0);
                 Ok(entries)
             }
             WAL_VERSION_CRC_UNCHAINED => {
@@ -477,15 +513,21 @@ fn chained_crc(entry_bytes: &[u8], prev_crc: u32) -> u32 {
 /// starting at the reader's current position, given the chain state to
 /// resume from (0 for a stream starting at the beginning of a fresh WAL).
 ///
-/// Returns the parsed entries and the final running CRC — the chain state to
+/// Returns the parsed entries, the final running CRC — the chain state to
-/// continue from for further appends. Stops (without erroring) at the first
+/// continue from for further appends — and the number of BYTES consumed by
-/// entry that fails to parse or whose stored CRC doesn't match the expected
+/// those verified entries. Stops (without erroring) at the first entry that
-/// chain value — a bit-flip, truncation/EOF, or an entry having been
+/// fails to parse or whose stored CRC doesn't match the expected chain value
+/// — a bit-flip, truncation/EOF, or an entry having been
 /// reordered/duplicated/spliced all produce a chain mismatch at that point,
 /// and are all handled the same way: replay stops there.
-fn read_chained_entries<R: Read>(f: &mut R, start_crc: u32) -> (Vec<WalEntry>, u32) {
+///
+/// The byte count is what lets `open()` position an append at the end of the
+/// VERIFIED prefix rather than at end-of-file. Appending past a torn tail
+/// writes entries that replay can never reach — see `open`.
+fn read_chained_entries<R: Read>(f: &mut R, start_crc: u32) -> (Vec<WalEntry>, u32, u64) {
     let mut entries = Vec::new();
     let mut running_crc = start_crc;
+    let mut verified_bytes: u64 = 0;
     loop {
         let raw_and_result = {
             let mut tee = TeeReader::new(f);
@@ -506,11 +548,14 @@ fn read_chained_entries<R: Read>(f: &mut R, start_crc: u32) -> (Vec<WalEntry>, u
             break;
         }
         running_crc = stored_crc;
+        // Only counted once the entry AND its CRC trailer verified, so the
+        // offset always points just past a complete, checked entry.
+        verified_bytes += raw.len() as u64 + crc_buf.len() as u64;
         if let Some(entry) = entry_opt {
             entries.push(entry);
         }
     }
-    (entries, running_crc)
+    (entries, running_crc, verified_bytes)
 }
 
 /// Create a fresh WAL file at `path` with the current-version header,
@@ -1052,6 +1097,51 @@ mod tests {
         assert_eq!(entries[0].chunk, "first");
     }
 
+    /// A crash mid-append leaves a torn final entry. Reopening the WAL must
+    /// place the next append at the end of the VERIFIED prefix, not at
+    /// end-of-file, or that append is written behind garbage the replay
+    /// scanner stops at — unreachable forever despite having returned Ok.
+    ///
+    /// This is the ordinary crash case, so getting it wrong loses
+    /// acknowledged writes in exactly the situation a WAL exists for.
+    #[test]
+    fn test_wal_append_after_torn_tail_stays_replayable() {
+        let dir = TempDir::new().unwrap();
+        let wal_path = dir.path().join("test.h5.wal");
+
+        let mut wal = WalFile::open(&wal_path).unwrap();
+        wal.append_save(&make_wal_entry("first", &[1.0, 2.0]))
+            .unwrap();
+        drop(wal);
+
+        // Simulate the crash: a partial entry appended after the good one.
+        {
+            use std::io::Write;
+            let mut f = std::fs::OpenOptions::new()
+                .append(true)
+                .open(&wal_path)
+                .unwrap();
+            f.write_all(&[0xAB, 0xCD, 0xEF, 0x01, 0x02]).unwrap();
+            f.flush().unwrap();
+        }
+
+        // Reopen and append. The torn bytes must not survive between the
+        // verified prefix and the new entry.
+        let mut wal = WalFile::open(&wal_path).unwrap();
+        wal.append_save(&make_wal_entry("second", &[3.0, 4.0]))
+            .unwrap();
+        drop(wal);
+
+        let entries = WalFile::read_entries(&wal_path).unwrap();
+        assert_eq!(
+            entries.len(),
+            2,
+            "the append after a torn tail must be replayable; got {} entr(y/ies) — \
+             the post-crash write was silently lost",
+            entries.len()
+        );
+    }
+
     /// Reordering two entries on disk must break the CRC chain — the
     /// second entry's stored CRC was computed against the first entry's
     /// real CRC, not against the chain state a reader sees after swapping