diff --git a/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs b/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs
index db50707e6..4461e0fd8 100644
--- a/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs
+++ b/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs
@@ -10,7 +10,9 @@ use std::io::{Read, Seek, SeekFrom};
use reed_solomon_erasure::galois_8::ReedSolomon;
-use crate::pb::volume_server_pb::{ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums};
+use crate::pb::volume_server_pb::{
+ ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums,
+};
use crate::storage::erasure_coding::ec_bitrot::{
self, ShardChecksumBuilder, DEFAULT_BITROT_BLOCK_SIZE,
};
@@ -82,9 +84,7 @@ pub fn write_ec_files(
// Write the generation-0 bitrot sidecar (`.ecsum`). Finalizing each
// builder yields covered_size (== total bytes written to that shard) and
- // the packed little-endian CRC32C array. Best-effort: a failed sidecar must
- // not fail the encode — the shards are already written and valid; the volume
- // simply runs with bitrot protection off for this generation.
+ // the packed little-endian CRC32C array.
let mut shard_checksums: Vec = Vec::with_capacity(total_shards);
for (i, builder) in builders.into_iter().enumerate() {
let (covered_size, packed) = builder.finalize();
@@ -107,6 +107,9 @@ pub fn write_ec_files(
};
let sidecar_path = ec_bitrot::bitrot_sidecar_path(&base, 0);
if let Err(e) = ec_bitrot::save_bitrot_sidecar(&sidecar_path, &prot) {
+ // A failed sidecar must not fail the encode — the shards are already
+ // written and valid. The volume simply runs with bitrot protection
+ // off for this generation until the sidecar is regenerated.
tracing::warn!(
volume_id = volume_id.0,
path = %sidecar_path,
@@ -695,8 +698,8 @@ fn encode_one_batch(
)
})?;
- // Write all shard buffers to files, feeding each shard's bytes into its
- // bitrot checksum builder so covered_size == the on-disk shard length.
+ // Write all shard buffers to files and feed the same bytes to each
+ // shard's bitrot checksum builder, keeping covered_size == on-disk length.
for (i, buf) in buffers.iter().enumerate() {
shards[i].write_all(buf)?;
builders[i].write(buf);
@@ -767,10 +770,7 @@ mod tests {
assert!(std::path::Path::new(&ecx_path).exists());
}
- /// Encode-time capture writes a valid generation-0 `.ecsum` sidecar whose
- /// recorded checksums match the actual on-disk shards.
- #[test]
- fn test_encode_writes_valid_bitrot_sidecar() {
+ fn make_volume_with_needles(n: u64) -> TempDir {
let tmp = TempDir::new().unwrap();
let dir = tmp.path().to_str().unwrap();
let mut v = Volume::new(
@@ -785,20 +785,32 @@ mod tests {
Version::current(),
)
.unwrap();
- for i in 1..=5 {
- let data = format!("test data for needle {}", i);
- let mut n = Needle {
+ for i in 1..=n {
+ // Larger payloads so encoded shards span multiple bitrot blocks
+ // would require huge data; small payloads are fine for correctness.
+ let data = format!("test data for needle {} {}", i, "x".repeat(64));
+ let mut needle = Needle {
id: NeedleId(i),
cookie: Cookie(i as u32),
data: data.as_bytes().to_vec(),
data_size: data.len() as u32,
..Needle::default()
};
- v.write_needle(&mut n, true).unwrap();
+ v.write_needle(&mut needle, true).unwrap();
}
v.sync_to_disk().unwrap();
v.close();
+ tmp
+ }
+ /// Encode-time capture writes a valid generation-0 `.ecsum` sidecar whose
+ /// recorded checksums match the actual on-disk shards.
+ #[test]
+ fn test_encode_writes_valid_bitrot_sidecar() {
+ use crate::storage::erasure_coding::ec_bitrot;
+
+ let tmp = make_volume_with_needles(5);
+ let dir = tmp.path().to_str().unwrap();
write_ec_files(dir, dir, "", VolumeId(1), 10, 4).unwrap();
let base = format!("{}/1", dir);
diff --git a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs
index 3a1c95c42..10df2040b 100644
--- a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs
+++ b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs
@@ -305,23 +305,40 @@ impl EcVolume {
(self.bitrot.clone(), self.bitrot_status)
}
- /// Verify every locally-held EC shard's raw bytes against the active-generation
- /// bitrot checksum sidecar. Read-only and purely diagnostic: it detects and
- /// reports corruption but never mutates or quarantines anything. It is the only
- /// path that exercises cold parity shards, never read during normal serving.
+ /// Read-only EC bitrot checksum scrub of the LOCAL shards of this volume's
+ /// active generation.
///
- /// Returns (blocks scanned, mismatched shard ids, errors). An absent/
- /// generation-mismatched sidecar is `Off` — a clean, empty no-op — so
- /// unprotected volumes are never reported broken. If more shards mismatch
- /// wholesale than parity can mask, the sidecar itself is the likely culprit
- /// (stale/wrong): the shard-corruption verdict is suppressed and an
- /// integrity note is added instead. Mirrors Go's `ChecksumScrub`.
+ /// Loads the active-generation `.ecsum` sidecar and, for each locally-held
+ /// shard, reads the on-disk shard file in `block_size` chunks and compares
+ /// each block's CRC32C against the sidecar. Returns
+ /// `(blocks_scanned, mismatched_shards, errors)`.
+ ///
+ /// If more than `parity_shards` shards mismatch wholesale (i.e. every block
+ /// of those shards is wrong — the signature of a stale/wrong sidecar rather
+ /// than localized disk rot), the result is classified as a suspect sidecar:
+ /// `mismatched_shards` is cleared and an integrity note is added to `errors`
+ /// instead. A genuine multi-shard disk failure of that magnitude is
+ /// already unrecoverable, so treating it as a sidecar-integrity issue avoids
+ /// raising false shard-corruption alarms.
+ ///
+ /// This method NEVER deletes or mutates anything — it is purely diagnostic.
pub fn checksum_scrub(&self) -> (u64, Vec, Vec) {
use crate::storage::erasure_coding::ec_bitrot;
use crate::storage::erasure_coding::ec_bitrot::BitrotStatus;
let mut errors: Vec = Vec::new();
+ // Resolve the active-generation protection AND its status, mirroring
+ // Go's `ChecksumScrub` (`prot, status := ecv.BitrotProtection()`):
+ // - BitrotOff => sidecars are OPTIONAL; an absent (or generation/
+ // config-mismatched) sidecar simply means protection is not enabled
+ // for this generation. Return a CLEAN, EMPTY result — NOT an error —
+ // so legacy/intentionally-unprotected volumes are never reported
+ // broken. (Go: `case BitrotOff: return 0, nil, nil`.)
+ // - BitrotInvalid => the sidecar is PRESENT but malformed/unverifiable
+ // (self-integrity or manifest failure). That is the only status that
+ // yields an integrity error here.
+ // - BitrotOn => scan local shards against it.
let prot = match self.bitrot_protection() {
(_, BitrotStatus::Off) => {
// Unprotected generation: nothing to verify. Not an error.
@@ -339,7 +356,8 @@ impl EcVolume {
}
(Some(p), BitrotStatus::On) => p,
(None, BitrotStatus::On) => {
- // Unreachable: BitrotOn always carries a loaded sidecar.
+ // Unreachable: BitrotOn always carries a loaded sidecar. Treat a
+ // missing payload defensively as protection off (clean no-op).
return (0, Vec::new(), Vec::new());
}
};
@@ -350,6 +368,8 @@ impl EcVolume {
let mut blocks_scanned: u64 = 0;
let mut mismatched_shards: Vec = Vec::new();
+ // Track shards whose blocks ALL mismatch (wholesale) to detect a
+ // stale/wrong sidecar.
let mut wholesale_mismatch = 0usize;
for (i, slot) in self.shards.iter().enumerate() {
@@ -365,6 +385,8 @@ impl EcVolume {
continue;
};
+ // Resolve the on-disk shard file path for the active generation,
+ // mirroring EcVolumeShard::reopen_against_generation's convention.
let path = if generation == 0 {
format!("{}.ec{:02}", base, shard_id)
} else {
@@ -391,9 +413,10 @@ impl EcVolume {
}
}
- // More wholesale mismatches than parity can mask => the sidecar itself is
- // suspect (stale generation / wrong volume): suppress the shard-corruption
- // verdict and flag a sidecar-integrity issue instead.
+ // If more shards mismatch wholesale than parity can mask, the sidecar
+ // itself is the likely culprit (stale generation / wrong volume), so
+ // suppress the shard-corruption verdict and flag a sidecar-integrity
+ // issue instead.
if wholesale_mismatch > self.parity_shards as usize {
errors.push(format!(
"EC volume {}: {} shards mismatch wholesale (> {} parity); suspect stale/wrong sidecar, not shard corruption",