volume server: ec.decode verifies, cleans up and compacts like Go, off the runtime (#11547)

* volume server: ec.decode reads the .ecx from the index dir it was copied to VolumeEcShardsCopy writes the .ecx/.ecj into the receiver's -dir.idx, so with a split data/index dir the decode target has no .ecx beside its shards. VolumeEcShardsToVolume sized the .dat from the right .ecx but built the .idx from the data dir, failing with NotFound after the .dat was already published. It now reads .ecx/.ecj from where the EC volume opened them and writes the .idx beside the .dat, where Go leaves it. The live-entry check and the .dat size also ignored deletions recorded only in the .ecj, which Go folds into the .ecx (RebuildEcxFile) first: a fully deleted volume was decoded instead of reported as having no live entries, and deleted tail needles were copied into the .dat. Both now treat journaled ids as deleted, without rewriting the sealed .ecx. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: ec.decode keeps the decoded volume writable and reads every .ecj The rebuilt .idx copied a journaled tail needle's .ecx row verbatim after the .dat was cut short before it, so the mount saw a row past EOF and marked the decoded volume read-only. Rows of deleted needles the .dat no longer holds are now dropped, and each journaled needle still in the .dat gets one tombstone instead of one per journal entry. VolumeEcShardsCopy appends journals collected from other holders into the idx dir, but the decode read only the .ecj beside the .ecx, which sits in the data dir when this server generated the shards. It now reads both, once, in bounded chunks via the loader EcVolume uses. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: test ec.decode drops a sealed .ecx tail tombstone Covers the other half of the rule added in the previous commit: a tail needle tombstoned in the .ecx itself (Go's RebuildEcxFile) is cut from the .dat, and its row must not reach the rebuilt .idx either. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: ec.decode runs its file I/O off the async runtime VolumeEcShardsToVolume released the store lock before decoding, but read the .ecx/.ecj, rebuilt the .dat and wrote the .idx inside the async handler, parking a runtime worker for the length of a volume-sized copy. The decode now runs in spawn_blocking on inputs snapshotted under the store lock. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: ec.decode checks the rebuilt .dat is complete Go stats the decoded .dat before writing the .idx (VerifyDecodedDatFile) and fails the decode when it is shorter than the extent the EC index references, since the caller deletes the shards once the call returns. The Rust handler returned success without that check. The rebuild already fails on a short shard read, so this guards the published file itself. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: ec.decode drops the decoded volume's bitrot sidecars Go removes <base>.ecsum and <base>.ecsum.v<N> beside the .dat and beside the .ecx once the .idx is written, so a stale checksum sidecar cannot pass for the protection of a later re-encode. The Rust handler left them in place. Removal is best effort, as in Go. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: ec.decode compacts the decoded volume Go ends VolumeEcShardsToVolume with an offline CompactVolumeFiles, so the decoded volume holds only live needles. The Rust decode left every needle deleted through the .ecj in the .dat, tombstoned in the .idx, until a later vacuum reclaimed it. Store::compact_volume_files loads the unmounted volume, checks free space the way the vacuum does (the estimate now lives in one helper), and runs the vacuum's compact-by-index and commit. As in Go a failed compaction is logged and the decode still succeeds, so the uncompacted .idx rules stay: the tests that pin them now make the compaction fail. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * volume server: ec.decode keeps deletes journaled while the .dat is written The decode read the .ecj journals once, before rebuilding the .dat, so a delete that reached the EC volume during the rebuild was left out of the new .idx and the needle came back live. Each journal's read length is now kept, and the bytes appended since are read just before the .idx is written, after waiting out any journal append in flight (appends hold the store write lock), so every delete acknowledged by then is in the .idx. A delete after that point is still lost, as in Go. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Guard overlapping ec decode requests; serialize journal catch-up volume_ec_shards_to_volume runs its decode in spawn_blocking, so a dropped request leaves the job running and a retry would race it on the temporary and final volume files. Claim the vid in a per-server in-flight set until the blocking job finishes, and return Unavailable to an overlapping request. The Go handler has the same exposure and gets the same guard. Journal appends hold the store write lock through their sync-or-truncate, so holding a read lock across the catch-up read guarantees every record it sees is committed: a rolled-back delete can no longer leave a tombstone in the decoded index. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * Reconcile the swap when offline compaction commit fails A CommitCompact that fails after the .cpc marker may have renamed .dat but not .idx. cleanup_compact refuses while the marker exists, so the mismatched pair survived until a restart reconciled it — and the decode caller treats the failure as non-fatal. Run reconcileCompactState on commit failure so a decided swap rolls forward and orphan temps are removed before the volume can mount. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * Release the decode claim on panic * volume: add ec_decodes_in_flight to the integration-test state literal * volume server: hold the decode tail's lock through compaction The catch_up read released before the rebuilt .idx was written and the volume compacted, so a delete synced to .ecj in that window was durably journaled yet absent from the published index — resurrecting the needle. Rust now holds the store read lock from catch_up through compact, and Go mirrors it by holding the volume's journal lock from the journal- consuming index write through CompactVolumeFiles. * volume server: serialize ec decode's tail per volume, not per store Review follow-ups on the decode path: - Rust: holding the store read lock from journal catch-up through the offline compaction stalled every writer on unrelated volumes for the whole rewrite. The new ec_decode_tail set marks the vid only while its .idx is published and .cpd/.cpx swapped; the two local .ecj append paths (VolumeEcBlobDelete, the distributed delete's local journal) wait on a Notify for that span — Go's per-volume ecjFileAccessLock semantics without the global stall. VolumeMount and the staged-adopt path are also held off while a decode claim is in flight so neither can race the swap. - Rust: the initial journal read ran unlocked, so bytes a rolled-back append later truncated could be folded in as phantom tombstones. The first pass stays unlocked (a slow journal must not stall the store) and a rescan under the quiescing read lock re-reads only committed content; catch_up now rebuilds the id set when a regular journal shrank. - Go: the decode resolved the compaction DiskLocation through FindEcVolume while holding the journal lock, inverting DestroyEcVolume's map->journal order into a deadlock. The lookup now happens first, and DestroyEcVolume/deleteEcVolumeById/DiskLocation.Close destroy outside the map lock. - Go: RebuildEcxFile unlinks .ecj while the volume's ecjFile handle stays open, so later deletes could commit to a detached inode. Both call sites now fold under the journal lock and ReopenDeletionJournal repoints the handle at the live path, working on the volume's resolved .ecx dir (EcIndexBaseFileName) rather than the configured index dir. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: fence EC remounts behind the destroy tombstone DestroyEcVolume, deleteEcVolumeById, and the collection-delete sweep now remove the EcVolume from ecVolumes before destroying it off-lock, so a concurrent remount could re-open shard files that the in-flight destroy then unlinks — registering a detached fd. Each destroy records a per-vid tombstone channel in a new ecVolumesDestroying map before dropping the map entry and closes it when Destroy returns. The tombstone intentionally survives as the vid's destroy generation: loadEcShardWithIdxDir compares it before and after opening the shard, so a destroy that both started and finished inside the open window is still detected. A mismatch drops the just-opened shard (releasing its fd and mount gauge) and retries after the destroy completes; a successful mount clears the stale tombstone. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: rescan the .ecj under the store lock only after a rollback The decode's second journal pass ran a full rescan under the store read lock on every decode, stalling unrelated writers for the length of the scan. Bump a process-wide epoch whenever a failed append truncates its uncommitted tail; an unchanged epoch between the unlocked read and the quiesced pass proves every id folded in was committed, so catch_up() suffices. catch_up() also treats a journal that was read but has since disappeared as shrunk to zero, so its earlier ids cannot linger. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: check the decode tail under the store write lock on delete A blob delete waited for the publishing tail before taking the store write lock, so a decode that claimed the tail while the delete was parked behind the decoder's read lock could still see the journal append land after the rebuilt .idx — an acknowledged delete the mount would miss. Test tail membership under the write lock instead, retrying after the wait; journal_delete_local reports WouldBlock for the same recheck on the distributed path. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: claim the vid for mount and staged adoption, per volume VolumeMount and the staged .copying adoption held the ec_decodes_in_flight set lock through slow file renames and mounts, stalling every unrelated volume's decode, mount, and adoption. Take the per-volume claim instead — the same exclusion against a racing decode for this vid, released when the call returns. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: fail the decode when a compaction commit marker survives CompactVolumeFiles' caller logged a compaction error and went on to delete the EC shards. When the commit marker (.cpc) is still on disk the .dat/.idx swap was decided but could not be reconciled, so the mounted pair may be mismatched — report the failure instead so the shards are kept and the caller can retry. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: gate the parked-delete test on the held write lock The releaser thread and the spawned delete raced for the store write lock; on a slow runner the delete could acquire it first and commit before the tail was ever claimed, failing !delete.is_finished() on the Windows unit-test job. Spawn the delete only after the thread reports the lock held. --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-20 04:54:24 +02:00 · 2026-10-03 14:55:15 +08:00
1 parent 68df7511f6
commit 07da302da0
@@ -97,21 +97,97 @@ pub fn read_ecj_deletions(
    collection: &str,
    volume_id: VolumeId,
 ) -> io::Result<HashSet<NeedleId>> {
-    let mut ids = HashSet::new();
-    for (i, dir) in dirs.iter().enumerate() {
-        if dirs[..i].contains(dir) {
-            continue;
+    Ok(EcjDeletions::read(dirs, collection, volume_id)?.ids)
+}
+
+/// The ids [`read_ecj_deletions`] returns, plus how far each journal was read
+/// so that ids journaled later can be added.
+pub struct EcjDeletions {
+    pub ids: HashSet<NeedleId>,
+    /// Each distinct journal path and the whole-record length read so far.
+    journals: Vec<(String, u64)>,
+}
+
+impl EcjDeletions {
+    pub fn read(dirs: &[&str], collection: &str, volume_id: VolumeId) -> io::Result<Self> {
+        let mut journals: Vec<(String, u64)> = Vec::new();
+        for dir in dirs {
+            let path = format!("{}.ecj", volume_file_name(dir, collection, volume_id));
+            if !journals.iter().any(|(p, _)| *p == path) {
+                journals.push((path, 0));
+            }
        }
-        let path = format!("{}.ecj", volume_file_name(dir, collection, volume_id));
-        let file = match File::open(&path) {
-            Ok(file) => file,
-            Err(e) if e.kind() == io::ErrorKind::NotFound => continue,
-            Err(e) => return Err(e),
+        let mut deletions = EcjDeletions {
+            ids: HashSet::new(),
+            journals,
        };
-        let len = file.metadata()?.len();
-        read_ecj_ids(&file, len, &mut ids)?;
+        deletions.catch_up()?;
+        Ok(deletions)
+    }
+
+    /// Adds the ids appended to each journal since the last read. A journal
+    /// that shrank is read again from the start — and the whole set rebuilt,
+    /// since ids already folded in from the truncated tail may have been a
+    /// rolled-back append. Non-regular journals (the FIFOs the tests stand
+    /// in for a blocking disk) stat empty and cannot be rolled back, so they
+    /// never count as shrunk.
+    pub fn catch_up(&mut self) -> io::Result<()> {
+        let mut shrank = false;
+        for (path, read_to) in &self.journals {
+            if *read_to == 0 {
+                continue;
+            }
+            match std::fs::metadata(path) {
+                Ok(m) => {
+                    if m.is_file() && m.len() < *read_to {
+                        shrank = true;
+                        break;
+                    }
+                }
+                // A journal that was read before and is now gone shrank to
+                // nothing (e.g. the volume was destroyed mid-scan) — the ids
+                // read from it no longer reflect committed content.
+                Err(e) if e.kind() == io::ErrorKind::NotFound => {
+                    shrank = true;
+                    break;
+                }
+                Err(e) => return Err(e),
+            }
+        }
+        if shrank {
+            self.ids.clear();
+            for (_, read_to) in &mut self.journals {
+                *read_to = 0;
+            }
+        }
+        for (path, read_to) in &mut self.journals {
+            let file = match File::open(&*path) {
+                Ok(file) => file,
+                Err(e) if e.kind() == io::ErrorKind::NotFound => continue,
+                Err(e) => return Err(e),
+            };
+            let len = file.metadata()?.len();
+            if len < *read_to {
+                *read_to = 0;
+            }
+            read_ecj_ids(&file, *read_to, len, &mut self.ids)?;
+            *read_to = len - len % NEEDLE_ID_SIZE as u64;
+        }
+        Ok(())
+    }
+
+    /// Clears the set and re-reads every journal from the start. Run under
+    /// the caller's store read lock (no append in flight): the result is
+    /// then exactly the committed content — an earlier unlocked read may
+    /// have folded in bytes a rolled-back append later truncated, or missed
+    /// a record re-appended to the very offset a rollback freed.
+    pub fn rescan(&mut self) -> io::Result<()> {
+        self.ids.clear();
+        for (_, read_to) in &mut self.journals {
+            *read_to = 0;
+        }
+        self.catch_up()
    }
-    Ok(ids)
 }

 /// What it takes to rebuild a volume's .dat from its EC data shards.
@@ -312,6 +388,30 @@ pub fn write_dat_file_from_shards(spec: &DatRebuild<'_>) -> io::Result<()> {
    write_result
 }

+/// Fails when the decoded `.dat` in `dat_dir` is shorter than the
+/// `dat_file_size` bytes its EC index references: the caller deletes the
+/// shards next, and they are the only other copy of the needles past the cut.
+/// A longer file passes.
+pub fn verify_decoded_dat_file(
+    dat_dir: &str,
+    collection: &str,
+    volume_id: VolumeId,
+    dat_file_size: i64,
+) -> io::Result<()> {
+    let dat_path = format!("{}.dat", volume_file_name(dat_dir, collection, volume_id));
+    let size = std::fs::metadata(&dat_path)?.len();
+    if (size as i64) < dat_file_size {
+        return Err(io::Error::new(
+            io::ErrorKind::UnexpectedEof,
+            format!(
+                "decoded {} is {} bytes, short of the {} its ec index references",
+                dat_path, size, dat_file_size
+            ),
+        ));
+    }
+    Ok(())
+}
+
 /// Write .idx file from .ecx index + .ecj deletion journal.
 ///
 /// See [`write_idx_file_from_ec_index_with_dirs`]; everything lives in `dir`.
@@ -790,4 +890,77 @@ mod tests {
        let expected: HashSet<NeedleId> = [1, 2, 3, 7, 9].into_iter().map(NeedleId).collect();
        assert_eq!(ids, expected);
    }
+
+    #[test]
+    fn test_verify_decoded_dat_file_rejects_a_short_dat() {
+        let tmp = TempDir::new().unwrap();
+        let dir = tmp.path().to_str().unwrap();
+        let dat_path = format!("{dir}/1.dat");
+
+        let err = verify_decoded_dat_file(dir, "", VolumeId(1), 100).unwrap_err();
+        assert_eq!(err.kind(), io::ErrorKind::NotFound);
+
+        std::fs::write(&dat_path, vec![0u8; 99]).unwrap();
+        let err = verify_decoded_dat_file(dir, "", VolumeId(1), 100).unwrap_err();
+        assert!(err.to_string().contains("short of the 100"), "{err}");
+
+        std::fs::write(&dat_path, vec![0u8; 100]).unwrap();
+        verify_decoded_dat_file(dir, "", VolumeId(1), 100).unwrap();
+        std::fs::write(&dat_path, vec![0u8; 101]).unwrap();
+        verify_decoded_dat_file(dir, "", VolumeId(1), 100).unwrap();
+    }
+
+    /// Ids appended after the first read, including the rest of a record torn
+    /// at that point, are picked up; a journal that shrank is read again.
+    #[test]
+    fn test_ecj_deletions_catch_up_reads_appended_ids() {
+        let tmp = TempDir::new().unwrap();
+        let dir = tmp.path().to_str().unwrap();
+        let ecj_path = format!("{dir}/1.ecj");
+        let entry = |id: u64| {
+            let mut buf = [0u8; NEEDLE_ID_SIZE];
+            NeedleId(id).to_bytes(&mut buf);
+            buf
+        };
+        let append = |bytes: &[u8]| {
+            let mut f = std::fs::OpenOptions::new()
+                .create(true)
+                .append(true)
+                .open(&ecj_path)
+                .unwrap();
+            f.write_all(bytes).unwrap();
+        };
+        let ids = |d: &EcjDeletions| {
+            let mut ids: Vec<u64> = d.ids.iter().map(|id| id.0).collect();
+            ids.sort();
+            ids
+        };
+
+        // No journal yet.
+        let mut deletions = EcjDeletions::read(&[dir, dir], "", VolumeId(1)).unwrap();
+        assert!(deletions.ids.is_empty());
+
+        append(&entry(1));
+        append(&entry(2)[..3]);
+        deletions.catch_up().unwrap();
+        assert_eq!(ids(&deletions), [1]);
+
+        append(&entry(2)[3..]);
+        append(&entry(3));
+        deletions.catch_up().unwrap();
+        assert_eq!(ids(&deletions), [1, 2, 3]);
+
+        // A shrunk journal is rebuilt from its surviving content: ids folded
+        // in from the truncated tail may have been rolled back and must not
+        // linger as phantom tombstones.
+        std::fs::write(&ecj_path, entry(9)).unwrap();
+        deletions.catch_up().unwrap();
+        assert_eq!(ids(&deletions), [9]);
+
+        // A journal removed since it was read is the extreme shrink: its
+        // earlier ids must go with it, not linger.
+        std::fs::remove_file(&ecj_path).unwrap();
+        deletions.catch_up().unwrap();
+        assert!(deletions.ids.is_empty());
+    }
 }
@@ -49,6 +49,15 @@ pub(crate) struct ShardLocationCache {
 /// A multiple of `NEEDLE_ID_SIZE`; 1 MiB is 131072 entries per syscall.
 const ECJ_LOAD_CHUNK_BYTES: usize = 1 << 20;

+/// Process-wide epoch bumped whenever a failed .ecj append truncates its
+/// uncommitted tail. A decode that read the journal without the store lock
+/// compares a sample taken before the read with one taken under the
+/// quiescing lock: equal means no rollback ran in between, so every id it
+/// folded in was committed and only incremental catch-up remains — the
+/// full rescan is needed only when the epoch moved.
+pub(crate) static ECJ_ROLLBACK_EPOCH: std::sync::atomic::AtomicU64 =
+    std::sync::atomic::AtomicU64::new(0);
+
 /// A `.ecj` smaller than this is never rewritten, however redundant. Below a
 /// megabyte the duplication costs nothing and the rewrite is pure churn.
 const ECJ_COMPACT_MIN_BYTES: i64 = 1 << 20;
@@ -134,15 +143,16 @@ enum EcjPublishError {
    HandleLost(io::Error),
 }

-/// Adds every whole needle id in the first `len` bytes of `ecj_file` to `ids`,
+/// Adds every whole needle id in bytes `from..len` of `ecj_file` to `ids`,
 /// reading `ECJ_LOAD_CHUNK_BYTES` at a time; a trailing partial record is
-/// ignored.
+/// ignored. `from` is a record boundary.
 pub(crate) fn read_ecj_ids(
    ecj_file: &File,
+    from: u64,
    len: u64,
    ids: &mut HashSet<NeedleId>,
 ) -> io::Result<()> {
-    read_ecj_ids_with(read_exact_at, ecj_file, len, ids)
+    read_ecj_ids_with(read_exact_at, ecj_file, from, len, ids)
 }

 /// [`read_ecj_ids`] with the positional read supplied by the caller, so the
@@ -150,11 +160,13 @@ pub(crate) fn read_ecj_ids(
 fn read_ecj_ids_with(
    read_at: fn(&File, &mut [u8], u64) -> io::Result<()>,
    ecj_file: &File,
+    from: u64,
    len: u64,
    ids: &mut HashSet<NeedleId>,
 ) -> io::Result<()> {
-    let mut buf = vec![0u8; std::cmp::min(ECJ_LOAD_CHUNK_BYTES as u64, len) as usize];
-    let mut off: u64 = 0;
+    let mut buf =
+        vec![0u8; std::cmp::min(ECJ_LOAD_CHUNK_BYTES as u64, len.saturating_sub(from)) as usize];
+    let mut off: u64 = from;
    while off + NEEDLE_ID_SIZE as u64 <= len {
        let mut want = std::cmp::min(ECJ_LOAD_CHUNK_BYTES as u64, len - off) as usize;
        want -= want % NEEDLE_ID_SIZE;
@@ -987,7 +999,7 @@ impl EcVolume {
        // held the `deleted_needles` write lock for the whole scan, which on a
        // bloated journal is the entire (unbounded) startup.
        let mut loaded: HashSet<NeedleId> = HashSet::new();
-        read_ecj_ids_with(read_at, ecj_file, self.ecj_file_size as u64, &mut loaded)?;
+        read_ecj_ids_with(read_at, ecj_file, 0, self.ecj_file_size as u64, &mut loaded)?;

        let mut set = self
            .deleted_needles
@@ -2038,6 +2050,11 @@ impl EcVolume {
                let ecj_path = self.ecj_file_name();
                let rollback = open_volume_file(OpenOptions::new().write(true), &ecj_path)
                    .and_then(|f| f.set_len(prev_ecj_size as u64).and_then(|_| f.sync_all()));
+                // Bumped on attempt, not success: a failed rollback leaves the
+                // bytes in place and a decode re-reading for the epoch change
+                // simply sees them — harmless — while a successful one must
+                // never go unnoticed by an unlocked journal read.
+                ECJ_ROLLBACK_EPOCH.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
                if let Err(trunc_err) = rollback {
                    tracing::error!(
                        volume_id = self.volume_id.0,