Issue #101: catalog.log: cut a partial append back before retrying
A failed append (disk full, dropped share) could leave part of its batch in catalog.log; the retry (same seqs) then shared a line with the fragment and the next load treated it as mid-log damage, setting later edits aside. - Journal::append cuts the log back to the last whole record on error (or, if that fails too, before the next append). - FsStore drops a failed append handle so the next attempt reopens the file; truncate of a never-created file is a no-op. - The loader recognises a CRC-valid record glued after a fragment (logs written by older builds) and skips only the fragment. - log_bytes now counts the newline added to an unterminated last record. - close_library writes the closing snapshot even when appending fails; the snapshot absorbs the queued ops (Journal::snapshot_with_unlogged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
572b55a44a
commit
08c2acda26
|
|
@ -10,6 +10,12 @@
|
|||
//! Loading = snapshot + replay of the records with `seq > snapshot seq`. Recovery rules:
|
||||
//! - a **torn final record** (crash mid-append: truncated line or bad CRC at the end) is dropped
|
||||
//! and the file is cut back to the last good record, so later appends start on a clean line;
|
||||
//! - a **failed append** (disk full, a dropped network share) may leave part of its batch in the
|
||||
//! file: [`Journal::append`] cuts the log back to the last whole record before returning the
|
||||
//! error (or, if that fails too, before the next append), so the retry — the same ops with the
|
||||
//! same `seq`s — starts on a clean line. Logs written by builds without that cut-back hold the
|
||||
//! fragment glued to the first retried record on one line; the loader recognises the whole,
|
||||
//! CRC-valid record at the end of such a line and skips only the fragment;
|
||||
//! - records already covered by the snapshot (crash between writing the snapshot and resetting
|
||||
//! the log) are skipped;
|
||||
//! - a bad record **followed by good ones** is real damage: replay stops there, the log is kept
|
||||
|
|
@ -62,6 +68,9 @@ pub struct LoadReport {
|
|||
pub failed: usize,
|
||||
/// Bytes of a torn final record that were dropped.
|
||||
pub torn_bytes: u64,
|
||||
/// Partial records left mid-log by a failed append that was retried (written by builds that
|
||||
/// didn't cut the log back after a failed write): skipped, the retried records were replayed.
|
||||
pub torn_fragments: usize,
|
||||
/// The damaged log was preserved under this name.
|
||||
pub damaged: Option<String>,
|
||||
/// No catalog files existed (a new library).
|
||||
|
|
@ -140,6 +149,9 @@ pub struct Journal {
|
|||
pending: Option<Pending>,
|
||||
/// After a failed background snapshot: don't retry before the log has this many records.
|
||||
retry_at_records: u64,
|
||||
/// A failed append may have left part of its records after `log_bytes`, and cutting them off
|
||||
/// failed too: the next append cuts the log back to `log_bytes` first.
|
||||
tail_dirty: bool,
|
||||
}
|
||||
|
||||
/// A background snapshot in flight.
|
||||
|
|
@ -182,6 +194,18 @@ pub fn decode_record(line: &str) -> Option<(u64, Op)> {
|
|||
serde_json::from_str(body).ok().map(|op| (seq, op))
|
||||
}
|
||||
|
||||
/// A line that doesn't decode but ends with a whole record: a fragment of a failed append followed
|
||||
/// by the record that retried it (the retry re-encodes the same ops with the same `seq`s). Returns
|
||||
/// the fragment's length and the record, if that record continues the log (`seq <= last + 1`).
|
||||
/// The record's CRC must match, so damage is never mistaken for it.
|
||||
fn resync(line: &[u8], last: u64) -> Option<(usize, (u64, Op))> {
|
||||
const START: &[u8] = b"{\"seq\":";
|
||||
(1..line.len().saturating_sub(START.len() - 1)).filter(|&i| line[i..].starts_with(START)).find_map(|i| {
|
||||
let rec = std::str::from_utf8(&line[i..]).ok().map(|l| l.trim_end_matches('\r')).and_then(decode_record)?;
|
||||
(rec.0 <= last + 1).then_some((i, rec))
|
||||
})
|
||||
}
|
||||
|
||||
/// Write `catalog.snap` for the state after op `seq`, streaming the JSON into the store (no
|
||||
/// whole-file string). The bytes are exactly
|
||||
/// `{"format":"lightcraft-catalog","version":1,"seq":N,"catalog":<serde_json of the catalog>}\n`,
|
||||
|
|
@ -242,6 +266,7 @@ impl Journal {
|
|||
stats: PersistStats::default(),
|
||||
pending: None,
|
||||
retry_at_records: 0,
|
||||
tail_dirty: false,
|
||||
};
|
||||
let log = log.unwrap_or_default();
|
||||
|
||||
|
|
@ -261,7 +286,17 @@ impl Journal {
|
|||
good_end = next;
|
||||
continue;
|
||||
}
|
||||
match line.and_then(decode_record) {
|
||||
let mut record = line.and_then(decode_record);
|
||||
if record.is_none()
|
||||
&& let Some((at, r)) = resync(&log[pos..line_end], j.seq)
|
||||
{
|
||||
// the torn start of a failed append, then the retried records (older builds
|
||||
// appended them right after the fragment)
|
||||
log::warn!("catalog log: skipped a partial record ({at} bytes) left by a failed write");
|
||||
report.torn_fragments += 1;
|
||||
record = Some(r);
|
||||
}
|
||||
match record {
|
||||
Some((seq, op)) if seq <= j.seq => {
|
||||
// already in the snapshot
|
||||
let _ = op;
|
||||
|
|
@ -301,10 +336,13 @@ impl Journal {
|
|||
report.torn_bytes = (log.len() - good_end) as u64;
|
||||
log::warn!("catalog log: dropped a torn final record ({} bytes)", report.torn_bytes);
|
||||
j.store.truncate(LOG, good_end as u64).map_err(io)?;
|
||||
j.log_bytes = good_end as u64;
|
||||
} else if needs_newline {
|
||||
j.store.append(LOG, b"\n").map_err(io)?;
|
||||
j.log_bytes = good_end as u64 + 1;
|
||||
} else {
|
||||
j.log_bytes = good_end as u64;
|
||||
}
|
||||
j.log_bytes = good_end as u64;
|
||||
}
|
||||
catalog.revision = 0;
|
||||
Ok((j, catalog, report))
|
||||
|
|
@ -323,7 +361,20 @@ impl Journal {
|
|||
buf.push_str(&encode_record(seq, op));
|
||||
buf.push('\n');
|
||||
}
|
||||
self.store.append(LOG, buf.as_bytes()).map_err(io)?;
|
||||
if self.tail_dirty {
|
||||
self.store.truncate(LOG, self.log_bytes).map_err(io)?;
|
||||
self.tail_dirty = false;
|
||||
}
|
||||
if let Err(e) = self.store.append(LOG, buf.as_bytes()) {
|
||||
// Part of the batch may be in the file. Cut it back to the last whole record, so the
|
||||
// retry (same seqs) starts on a clean line instead of after a fragment that would
|
||||
// read back as damage.
|
||||
if let Err(t) = self.store.truncate(LOG, self.log_bytes) {
|
||||
log::error!("catalog: can't cut the log back after a failed append: {t}");
|
||||
self.tail_dirty = true;
|
||||
}
|
||||
return Err(io(e));
|
||||
}
|
||||
if let Some(p) = self.pending.as_mut() {
|
||||
p.tail.extend_from_slice(buf.as_bytes());
|
||||
p.tail_records += ops.len() as u64;
|
||||
|
|
@ -343,12 +394,22 @@ impl Journal {
|
|||
|
||||
/// Write a snapshot of `catalog` (which must reflect every appended op) and reset the log.
|
||||
pub fn snapshot(&mut self, catalog: &Catalog) -> Result<()> {
|
||||
self.snapshot_with_unlogged(catalog, 0)
|
||||
}
|
||||
|
||||
/// [`Journal::snapshot`] of a catalog that also reflects `unlogged` ops applied after the
|
||||
/// last appended one but never written to the log (their append failed). The snapshot counts
|
||||
/// them (it holds the state after op `seq + unlogged`), so they are saved and must not be
|
||||
/// appended afterwards. Once the snapshot is durable [`Journal::seq`] includes them, even if
|
||||
/// resetting the log then fails (the error is still returned).
|
||||
pub fn snapshot_with_unlogged(&mut self, catalog: &Catalog, unlogged: u64) -> Result<()> {
|
||||
// never two snapshot writers; the one in flight is older, so it must land first
|
||||
if let Err(e) = self.wait() {
|
||||
log::warn!("catalog: background snapshot failed ({e}); writing one now");
|
||||
}
|
||||
let t0 = web_time::Instant::now();
|
||||
let mut timing = match write_snapshot(self.store.as_mut(), self.seq, catalog) {
|
||||
let seq = self.seq + unlogged;
|
||||
let mut timing = match write_snapshot(self.store.as_mut(), seq, catalog) {
|
||||
Ok(t) => t,
|
||||
Err(e) => {
|
||||
// nothing lost (the log is whole); don't retry on every append
|
||||
|
|
@ -357,6 +418,9 @@ impl Journal {
|
|||
return Err(io(e));
|
||||
}
|
||||
};
|
||||
// the snapshot is durable: it holds every op up to `seq`
|
||||
self.seq = seq;
|
||||
self.snapshot_seq = seq;
|
||||
let t2 = web_time::Instant::now();
|
||||
// A crash here leaves old records in the log; they are skipped by seq on load.
|
||||
self.store.write_atomic(LOG, b"").map_err(io)?;
|
||||
|
|
@ -365,10 +429,10 @@ impl Journal {
|
|||
timing.blocking_ms = timing.total_ms;
|
||||
timing.records = self.log_records;
|
||||
self.record_snapshot(timing);
|
||||
self.snapshot_seq = self.seq;
|
||||
self.log_records = 0;
|
||||
self.log_bytes = 0;
|
||||
self.retry_at_records = 0;
|
||||
self.tail_dirty = false;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
@ -459,6 +523,7 @@ impl Journal {
|
|||
self.log_records = p.tail_records;
|
||||
self.log_bytes = p.tail.len() as u64;
|
||||
self.retry_at_records = 0;
|
||||
self.tail_dirty = false;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -646,3 +646,5 @@ mod tests_background;
|
|||
mod tests_journal;
|
||||
#[cfg(test)]
|
||||
mod tests_local;
|
||||
#[cfg(test)]
|
||||
mod tests_torn_append;
|
||||
|
|
|
|||
|
|
@ -28,9 +28,12 @@ pub trait Store: Send {
|
|||
self.write_atomic(name, &buf)?;
|
||||
Ok(buf.len() as u64)
|
||||
}
|
||||
/// Append and make durable (fsync) before returning.
|
||||
/// Append and make durable (fsync) before returning. On an error, part of `data` may have
|
||||
/// been written (a full disk, a dropped network share): the journal cuts the file back to its
|
||||
/// previous length with [`Store::truncate`] before appending again.
|
||||
fn append(&mut self, name: &str, data: &[u8]) -> io::Result<()>;
|
||||
/// Cut a file to `len` bytes (drop a torn tail before appending again).
|
||||
/// Cut a file to `len` bytes (drop a torn tail before appending again). A missing file is
|
||||
/// left missing.
|
||||
fn truncate(&mut self, name: &str, len: u64) -> io::Result<()>;
|
||||
/// Human-readable location (diagnostics).
|
||||
fn describe(&self) -> String;
|
||||
|
|
@ -132,15 +135,26 @@ impl Store for FsStore {
|
|||
self.appender = Some((name.to_string(), f));
|
||||
}
|
||||
let Some((_, f)) = self.appender.as_mut() else { return Err(io::Error::other("appender not open")) };
|
||||
f.write_all(data)?;
|
||||
f.sync_data()
|
||||
let written = f.write_all(data).and_then(|()| f.sync_data());
|
||||
if written.is_err() {
|
||||
// The handle may be dead (a network share that reconnected, a removed volume): the next
|
||||
// attempt reopens the file. Part of `data` may be in the file; the caller cuts it back
|
||||
// ([`Store::truncate`]) before appending again.
|
||||
self.appender = None;
|
||||
}
|
||||
written
|
||||
}
|
||||
|
||||
fn truncate(&mut self, name: &str, len: u64) -> io::Result<()> {
|
||||
if self.appender.as_ref().is_some_and(|(n, _)| n == name) {
|
||||
self.appender = None;
|
||||
}
|
||||
let f = std::fs::OpenOptions::new().write(true).open(self.path(name))?;
|
||||
let f = match std::fs::OpenOptions::new().write(true).open(self.path(name)) {
|
||||
Ok(f) => f,
|
||||
// nothing to cut (an append that failed before creating the file)
|
||||
Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(e) => return Err(e),
|
||||
};
|
||||
f.set_len(len)?;
|
||||
f.sync_all()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,223 @@
|
|||
//! Issue #101: an append that fails part-way (disk full, a dropped network share) and is then
|
||||
//! retried must not read back as log damage, and must lose nothing.
|
||||
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use crate::journal::LOG;
|
||||
use crate::*;
|
||||
|
||||
/// A store whose next append writes only a prefix of its data, then fails.
|
||||
#[derive(Clone, Default)]
|
||||
struct Torn {
|
||||
files: MemStore,
|
||||
/// The next append writes this many bytes, then returns an error.
|
||||
torn_at: Arc<Mutex<Option<usize>>>,
|
||||
/// `truncate` fails (the cut-back after a failed append can't be done right away).
|
||||
truncate_fails: Arc<AtomicBool>,
|
||||
}
|
||||
|
||||
impl Torn {
|
||||
fn tear_next_append(&self, bytes: usize) {
|
||||
*self.torn_at.lock().unwrap() = Some(bytes);
|
||||
}
|
||||
}
|
||||
|
||||
impl Store for Torn {
|
||||
fn read(&mut self, name: &str) -> std::io::Result<Option<Vec<u8>>> {
|
||||
self.files.read(name)
|
||||
}
|
||||
fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> {
|
||||
self.files.write_atomic(name, data)
|
||||
}
|
||||
fn append(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> {
|
||||
if let Some(n) = self.torn_at.lock().unwrap().take() {
|
||||
self.files.append(name, &data[..n.min(data.len())])?;
|
||||
return Err(std::io::Error::other("disk full"));
|
||||
}
|
||||
self.files.append(name, data)
|
||||
}
|
||||
fn truncate(&mut self, name: &str, len: u64) -> std::io::Result<()> {
|
||||
if self.truncate_fails.load(Ordering::SeqCst) {
|
||||
return Err(std::io::Error::other("share unavailable"));
|
||||
}
|
||||
self.files.truncate(name, len)
|
||||
}
|
||||
fn describe(&self) -> String {
|
||||
"torn".into()
|
||||
}
|
||||
}
|
||||
|
||||
fn add_photo(c: &mut Catalog) -> Op {
|
||||
let id = c.alloc_photo_id();
|
||||
Op::AddPhoto { photo: Box::new(Photo::new(id, Source::File { path: format!("/p/{}.jpg", id.0) }, "p.jpg", "JPEG", 4, 3, "2026-01-01")) }
|
||||
}
|
||||
|
||||
/// Apply `ops` to the live catalog (like a command) and return them for the journal.
|
||||
fn apply(c: &mut Catalog, ops: Vec<Op>) -> Vec<Op> {
|
||||
for op in &ops {
|
||||
c.apply(op.clone()).unwrap();
|
||||
}
|
||||
ops
|
||||
}
|
||||
|
||||
/// Every tear position of a batch, then the retry (same ops, same seqs, like the engine's queue)
|
||||
/// and more appends: reloading replays everything, reports no damage and no torn bytes.
|
||||
#[test]
|
||||
fn partial_append_then_retry_loads_cleanly() {
|
||||
let probe = MemStore::new();
|
||||
let (mut j, mut c, _) = Journal::open(Box::new(probe.clone())).unwrap();
|
||||
let add = add_photo(&mut c);
|
||||
let first = apply(&mut c, vec![add]);
|
||||
j.append(&first).unwrap();
|
||||
let id = c.photos().next().unwrap().id;
|
||||
let batch = apply(&mut c, vec![Op::SetRating { id, rating: 3 }, Op::SetFlag { id, flag: Flag::Pick }, Op::SetRating { id, rating: 4 }]);
|
||||
let before = probe.get(LOG).unwrap().len();
|
||||
j.append(&batch).unwrap();
|
||||
let batch_len = probe.get(LOG).unwrap().len() - before;
|
||||
|
||||
for tear in 1..batch_len {
|
||||
let t = Torn::default();
|
||||
let (mut j, mut c, _) = Journal::open(Box::new(t.clone())).unwrap();
|
||||
let add = add_photo(&mut c);
|
||||
let first = apply(&mut c, vec![add]);
|
||||
j.append(&first).unwrap();
|
||||
let good_len = t.files.get(LOG).unwrap().len();
|
||||
let batch = apply(&mut c, vec![Op::SetRating { id, rating: 3 }, Op::SetFlag { id, flag: Flag::Pick }, Op::SetRating { id, rating: 4 }]);
|
||||
t.tear_next_append(tear);
|
||||
assert!(j.append(&batch).is_err(), "tear at {tear}");
|
||||
// cut back to the last whole record
|
||||
assert_eq!(t.files.get(LOG).unwrap().len(), good_len, "tear at {tear}");
|
||||
j.append(&batch).unwrap();
|
||||
let more = apply(&mut c, vec![Op::SetLabel { id, label: Some(ColorLabel::ALL[0]) }]);
|
||||
j.append(&more).unwrap();
|
||||
drop(j);
|
||||
|
||||
let (_, loaded, r) = Journal::open(Box::new(t.clone())).unwrap();
|
||||
assert!(r.damaged.is_none(), "tear at {tear}: {r:?}");
|
||||
assert_eq!((r.torn_bytes, r.torn_fragments, r.replayed), (0, 0, 5), "tear at {tear}");
|
||||
assert_eq!(loaded.to_snapshot(), c.to_snapshot(), "tear at {tear}");
|
||||
}
|
||||
}
|
||||
|
||||
/// When cutting back fails too (the share is gone), the next append cuts back first; until that
|
||||
/// works nothing more is appended after the fragment.
|
||||
#[test]
|
||||
fn failed_cut_back_is_done_before_the_next_append() {
|
||||
let t = Torn::default();
|
||||
let (mut j, mut c, _) = Journal::open(Box::new(t.clone())).unwrap();
|
||||
let add = add_photo(&mut c);
|
||||
let first = apply(&mut c, vec![add]);
|
||||
j.append(&first).unwrap();
|
||||
let good_len = t.files.get(LOG).unwrap().len();
|
||||
let id = c.photos().next().unwrap().id;
|
||||
let batch = apply(&mut c, vec![Op::SetRating { id, rating: 2 }, Op::SetRating { id, rating: 5 }]);
|
||||
t.truncate_fails.store(true, Ordering::SeqCst);
|
||||
t.tear_next_append(30);
|
||||
assert!(j.append(&batch).is_err());
|
||||
assert_eq!(t.files.get(LOG).unwrap().len(), good_len + 30, "the fragment is still there");
|
||||
// still unreachable: the retry fails without writing anything after the fragment
|
||||
assert!(j.append(&batch).is_err());
|
||||
assert_eq!(t.files.get(LOG).unwrap().len(), good_len + 30);
|
||||
t.truncate_fails.store(false, Ordering::SeqCst);
|
||||
j.append(&batch).unwrap();
|
||||
drop(j);
|
||||
let (_, loaded, r) = Journal::open(Box::new(t.clone())).unwrap();
|
||||
assert!(r.damaged.is_none(), "{r:?}");
|
||||
assert_eq!(r.replayed, 3);
|
||||
assert_eq!(loaded.to_snapshot(), c.to_snapshot());
|
||||
}
|
||||
|
||||
/// Logs written before the fix: the fragment of the failed append is glued to the first retried
|
||||
/// record on one line, followed by the rest of the retry. They load in full (the fragment is
|
||||
/// skipped), instead of stopping at the fragment and setting the later records aside as damage.
|
||||
#[test]
|
||||
fn legacy_fragment_glued_to_the_retry_is_skipped() {
|
||||
let m = MemStore::new();
|
||||
let (mut j, mut c, _) = Journal::open(Box::new(m.clone())).unwrap();
|
||||
let add = add_photo(&mut c);
|
||||
let first = apply(&mut c, vec![add]);
|
||||
j.append(&first).unwrap();
|
||||
let good = m.get(LOG).unwrap();
|
||||
let id = c.photos().next().unwrap().id;
|
||||
let batch = apply(
|
||||
&mut c,
|
||||
vec![
|
||||
Op::SetRating { id, rating: 3 },
|
||||
Op::SetFlag { id, flag: Flag::Pick },
|
||||
Op::SetLabelName { label: ColorLabel::ALL[1], name: Some("Größe ✓".into()) },
|
||||
],
|
||||
);
|
||||
j.append(&batch).unwrap();
|
||||
let full = m.get(LOG).unwrap();
|
||||
let batch_bytes = &full[good.len()..];
|
||||
let later = apply(&mut c, vec![Op::SetRating { id, rating: 1 }]);
|
||||
j.append(&later).unwrap();
|
||||
let later_bytes = m.get(LOG).unwrap()[full.len()..].to_vec();
|
||||
drop(j);
|
||||
|
||||
// every tear (inside the first record, after it, inside a multi-byte character…)
|
||||
for tear in 1..batch_bytes.len() {
|
||||
if batch_bytes[tear - 1] == b'\n' {
|
||||
continue; // a tear on a record boundary leaves no fragment
|
||||
}
|
||||
let mut log = good.clone();
|
||||
log.extend_from_slice(&batch_bytes[..tear]);
|
||||
log.extend_from_slice(batch_bytes);
|
||||
log.extend_from_slice(&later_bytes);
|
||||
let t = MemStore::new();
|
||||
t.set(LOG, log);
|
||||
let (_, loaded, r) = Journal::open(Box::new(t)).unwrap();
|
||||
assert!(r.damaged.is_none(), "tear at {tear}: {r:?}");
|
||||
assert_eq!(r.torn_fragments, 1, "tear at {tear}");
|
||||
assert_eq!(loaded.to_snapshot(), c.to_snapshot(), "tear at {tear}");
|
||||
}
|
||||
|
||||
// a damaged record whose tail happens to be a whole record from the future is still damage
|
||||
let mut log = good.clone();
|
||||
log.extend_from_slice(b"garbage");
|
||||
log.extend_from_slice(&later_bytes);
|
||||
log.extend_from_slice(&later_bytes);
|
||||
let t = MemStore::new();
|
||||
t.set(LOG, log);
|
||||
let (_, _, r) = Journal::open(Box::new(t)).unwrap();
|
||||
assert!(r.damaged.is_some(), "{r:?}");
|
||||
}
|
||||
|
||||
/// A snapshot can absorb ops whose append failed: it records them as saved (`seq` moves past
|
||||
/// them), and appending after it continues from there.
|
||||
#[test]
|
||||
fn snapshot_with_unlogged_ops_saves_them() {
|
||||
let t = Torn::default();
|
||||
let (mut j, mut c, _) = Journal::open(Box::new(t.clone())).unwrap();
|
||||
let add = add_photo(&mut c);
|
||||
let first = apply(&mut c, vec![add]);
|
||||
j.append(&first).unwrap();
|
||||
let id = c.photos().next().unwrap().id;
|
||||
let queued = apply(&mut c, vec![Op::SetRating { id, rating: 2 }, Op::SetFlag { id, flag: Flag::Reject }]);
|
||||
t.tear_next_append(10);
|
||||
assert!(j.append(&queued).is_err());
|
||||
j.snapshot_with_unlogged(&c, queued.len() as u64).unwrap();
|
||||
assert_eq!(j.seq(), 3);
|
||||
let after = apply(&mut c, vec![Op::SetRating { id, rating: 5 }]);
|
||||
j.append(&after).unwrap();
|
||||
drop(j);
|
||||
let (j, loaded, r) = Journal::open(Box::new(t.clone())).unwrap();
|
||||
assert_eq!((r.snapshot_seq, r.replayed), (3, 1));
|
||||
assert_eq!(j.seq(), 4);
|
||||
assert_eq!(loaded.to_snapshot(), c.to_snapshot());
|
||||
}
|
||||
|
||||
/// `FsStore::truncate` on a file that was never created (an append that failed first) is a no-op.
|
||||
#[test]
|
||||
fn fs_store_truncates_a_missing_file_quietly() {
|
||||
let dir = std::env::temp_dir().join(format!("lc-torn-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
let mut s = FsStore::open(&dir).unwrap();
|
||||
s.truncate(LOG, 0).unwrap();
|
||||
assert!(!dir.join(LOG).exists());
|
||||
s.append(LOG, b"abc\n").unwrap();
|
||||
s.truncate(LOG, 0).unwrap();
|
||||
assert_eq!(std::fs::read(dir.join(LOG)).unwrap(), b"");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
|
@ -362,16 +362,35 @@ impl Session {
|
|||
}
|
||||
|
||||
/// Flush everything and write a snapshot (on quit). Ends an open interaction first.
|
||||
///
|
||||
/// The snapshot is written even when appending the queued ops fails: it is a fresh file
|
||||
/// holding everything in memory, so it saves those ops too (the log handle may be the only
|
||||
/// thing that's broken). Fails only if nothing could be saved; the ops then stay queued.
|
||||
pub fn close_library(&mut self) -> Result<()> {
|
||||
if self.library.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
let _ = self.end_interaction();
|
||||
self.persist()?;
|
||||
let persisted = self.persist();
|
||||
self.save_view();
|
||||
let unlogged = if persisted.is_err() { self.pending_log.len() as u64 } else { 0 };
|
||||
let Some(lib) = self.library.as_mut() else { return Ok(()) };
|
||||
lib.journal.snapshot(&self.catalog)?;
|
||||
Ok(())
|
||||
let before = lib.journal.seq();
|
||||
let snapshot = lib.journal.snapshot_with_unlogged(&self.catalog, unlogged);
|
||||
if unlogged > 0 && lib.journal.seq() == before + unlogged {
|
||||
// the snapshot holds the queued ops: they are saved, never append them again
|
||||
log::info!("library: {unlogged} queued change(s) saved in the closing snapshot");
|
||||
self.pending_log.clear();
|
||||
lib.unsaved_error = None;
|
||||
lib.retry_at = None;
|
||||
lib.last_error = None;
|
||||
}
|
||||
match (persisted, snapshot) {
|
||||
(_, Ok(())) => Ok(()),
|
||||
// neither the log nor the snapshot took the queued ops
|
||||
(Err(EngineError::NotSaved(e)), Err(s)) if lib.journal.seq() == before => Err(EngineError::NotSaved(format!("{e}; snapshot: {s}"))),
|
||||
(_, Err(s)) => Err(s.into()),
|
||||
}
|
||||
}
|
||||
|
||||
fn view_json(&self) -> Vec<u8> {
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ struct Flaky {
|
|||
fail: Arc<AtomicBool>,
|
||||
/// Only snapshots fail (appends work).
|
||||
snapshots_only: bool,
|
||||
/// Only appends fail (a dead log handle; fresh files can still be written).
|
||||
appends_only: bool,
|
||||
}
|
||||
|
||||
impl Flaky {
|
||||
|
|
@ -39,7 +41,7 @@ impl Store for Flaky {
|
|||
self.files.read(name)
|
||||
}
|
||||
fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> {
|
||||
if self.failing() {
|
||||
if self.failing() && !self.appends_only {
|
||||
return Err(std::io::Error::other("disk full"));
|
||||
}
|
||||
self.files.write_atomic(name, data)
|
||||
|
|
@ -145,3 +147,47 @@ fn failed_snapshot_does_not_fail_the_command() {
|
|||
store.set_failing(false);
|
||||
assert_eq!(open(&store, false).catalog.albums().count(), 2);
|
||||
}
|
||||
|
||||
/// Issue #101: quitting while the log can't be appended to still saves the queued changes in the
|
||||
/// closing snapshot (a fresh file), and reports success; they are not appended again later.
|
||||
#[test]
|
||||
fn close_saves_queued_changes_in_the_snapshot_when_appends_fail() {
|
||||
let store = Flaky { appends_only: true, ..Default::default() };
|
||||
let mut s = open(&store, true);
|
||||
let id = s.selection.active.unwrap();
|
||||
store.set_failing(true);
|
||||
assert!(matches!(s.execute("photo.rate", &json!({"rating": 4})), Err(EngineError::NotSaved(_))));
|
||||
assert!(matches!(s.execute("photo.flag", &json!({"flag": "pick"})), Err(EngineError::NotSaved(_))));
|
||||
assert_eq!(s.unsaved().map(|u| u.0), Some(2));
|
||||
s.close_library().unwrap();
|
||||
assert_eq!(s.unsaved(), None);
|
||||
assert!(s.pending_log.is_empty());
|
||||
let expect = s.catalog.to_snapshot();
|
||||
drop(s);
|
||||
store.set_failing(false);
|
||||
let mut s2 = open(&store, true);
|
||||
assert_eq!(s2.catalog.to_snapshot(), expect);
|
||||
let p = s2.catalog.photo(id).unwrap();
|
||||
assert_eq!((p.rating, p.flag), (4, Flag::Pick));
|
||||
// later edits continue after the absorbed ops
|
||||
s2.execute("photo.rate", &json!({"rating": 2})).unwrap();
|
||||
let expect = s2.catalog.to_snapshot();
|
||||
drop(s2);
|
||||
assert_eq!(open(&store, true).catalog.to_snapshot(), expect);
|
||||
}
|
||||
|
||||
/// When neither the log nor the snapshot can be written, closing fails with `NotSaved` and the
|
||||
/// changes stay queued (a later retry can still save them).
|
||||
#[test]
|
||||
fn close_fails_when_nothing_can_be_saved() {
|
||||
let store = Flaky::default();
|
||||
let mut s = open(&store, true);
|
||||
store.set_failing(true);
|
||||
assert!(s.execute("photo.rate", &json!({"rating": 4})).is_err());
|
||||
let e = s.close_library().unwrap_err();
|
||||
assert!(matches!(e, EngineError::NotSaved(_)), "{e:?}");
|
||||
assert_eq!(s.unsaved().map(|u| u.0), Some(1));
|
||||
store.set_failing(false);
|
||||
s.close_library().unwrap();
|
||||
assert_eq!(s.unsaved(), None);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -488,7 +488,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
|
|||
|
||||
| Id | Feature | Tier | Status | Evidence | Missing / notes |
|
||||
|---|---|---|---|---|---|
|
||||
| LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | every change fsynced before the command returns; a failed write fails the command ("saved in memory but not written to disk"), keeps the change queued and retries; top-bar warning until saved |
|
||||
| LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | every change fsynced before the command returns; a failed write fails the command ("saved in memory but not written to disk"), keeps the change queued and retries; top-bar warning until saved; a write that fails part-way is cut back to the last whole record so the retry never reads back as damage (logs from older builds with such a fragment load in full); quitting saves queued changes in the closing snapshot even when the log can't be appended to |
|
||||
| LR-BEHAV-UNDO | Global undo | P0 | ✅ | `cmd:edit.undo`, `crates/engine/src/tests.rs` (`rating_flag_undo_redo`) | covers ratings, albums, deletes, edits |
|
||||
| LR-BEHAV-MULTISELECT | Multi-selection | P0 | ✅ | `cmd:library.select` (`replace`/`add`/`toggle`/`range`), `cmd:library.selectAll` | |
|
||||
| LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | |
|
||||
|
|
|
|||
|
|
@ -10,6 +10,10 @@
|
|||
- 23 new built-in presets: Portrait, Landscape, Urban, Food, Seasons, Vintage and B&W toners.
|
||||
|
||||
### Reliability
|
||||
- A save that fails part-way (a full disk, a network share that drops) no longer looks like a damaged catalog
|
||||
afterwards (issue #101): the partial write is cut off before LightCraft retries, so the next launch replays every
|
||||
change. Catalogs already holding such a fragment load in full. Quitting while the catalog log can't be written
|
||||
still saves your queued changes in the closing snapshot.
|
||||
- Canon CR2 photos from the EOS 7D, 50D, 60D, 550D, 600D, 1200D, 1300D, 5D Mark II and 1D Mark IV (and other
|
||||
models whose sensor starts on a green-blue row) no longer come out magenta (issue #85): the colour-filter
|
||||
layout is read from each file instead of assumed.
|
||||
|
|
|
|||
Loading…
Reference in New Issue