style(ec): align bitrot comments and test setup for merge-cleanliness

Match the shared bitrot code (write_ec_files, encode_one_batch, checksum_scrub,
the encode sidecar test) to the canonical wording/layout so the volume-server
Rust port stays line-aligned across trees, keeping periodic merges conflict-free.
No behavior change.

Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo
This commit is contained in:
Chris Lu
2026-06-30 19:48:13 -07:00
parent 4d6fc8047e
commit 9bc216b9eb
2 changed files with 63 additions and 28 deletions
@@ -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 (`<base>.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<EcShardChecksums> = 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);
@@ -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<u32>, Vec<String>) {
use crate::storage::erasure_coding::ec_bitrot;
use crate::storage::erasure_coding::ec_bitrot::BitrotStatus;
let mut errors: Vec<String> = 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<u32> = 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",