From f0afcf904d7584449a8babb8e2f432a7ba490a3e Mon Sep 17 00:00:00 2001 From: Eliah Rusin Date: Thu, 24 Sep 2026 02:08:56 +0300 Subject: [PATCH] volume: an EC volume needs an .ecx to mount, and a 0-byte stub never outranks a real index (#11415) * volume: an EC volume needs a non-empty .ecx to mount Two gaps against Go in how the Rust volume server treats the .ecx. EcVolume::new mounted with no index at all. The per-shard VolumeEcShardsMount path picks the disk by shard file alone, so a shard whose .ecx was on no local directory still registered and was advertised to the master; every VolumeEcShardRead then failed with "ecx file not open", and add_shard's 0-byte guard was neutralised because ecx_file_size stayed 0. Go's NewEcVolume returns an error wrapping os.ErrNotExist. EcVolume::new now fails with NotFound, and Store::mount_ec_shard looks up the .ecx owner across all disks first (findEcxIdxDirForVolume) so a shard on a sibling disk of its index still mounts instead of turning into a hard failure. A 0-byte .ecx stub, as left by a failed EC distribute copy, counted as a valid index. Go requires Size() > 0 wherever the file steers a decision: HasEcxFileOnDisk, findEcxIdxDirForVolume, indexEcxOwners (shared by reconcile and mirror), and VolumeEcShardsCopy removes a copied 0-byte .ecx and fails the copy. Mirror each through one is_usable_ecx_file helper. NewEcVolume itself still accepts a lone 0-byte .ecx as a legitimate empty index, but prefers a non-empty copy, local directory first, over a stub in the other directory; the resolution in EcVolume::new now follows the same order. Tests that mounted EC volumes without any .ecx get a real fixture. Co-Authored-By: Claude Fable 5.1 * volume: mount_ec_shard tries every disk; reconcile ignores a 0-byte local .ecx mount_ec_shard returned the first disk's error, so an unusable shard copy (a 0-byte .ecNN left by an interrupted move) hid a good copy on the next disk. Like Go's MountEcShards, keep scanning: NotFound means "not this disk", any other failure is collected, and an all-disks-fail error names every disk tried. "No .ecx on any local disk" is now told apart from "shard not on this server". The orphan-shard reconcile took its locally-mirrored fast path whenever a local .ecx existed at all. A 0-byte stub there registered the shards against an empty index while the owner index skipped that same stub. Go gates the fast path on HasEcxFileOnDisk; do the same. ec_local_ecx_path loses its last production caller and becomes test-only. Co-Authored-By: Claude Fable 5.1 * volume: match Go's mount error text and skip the owner stat on the owning disk MountEcShards in Go skips the HasEcxFileOnDisk stat when the disk's own directories already hold the .ecx, dedups a shared -dir.idx across locations in findEcxIdxDirForVolume, and reports "load failures" with the same wording. Also drop two issue-number references from comments. --------- Co-authored-by: Claude Fable 5.1 Co-authored-by: Chris Lu --- seaweed-volume/src/server/grpc_server.rs | 13 +- seaweed-volume/src/server/heartbeat.rs | 6 + seaweed-volume/src/storage/disk_location.rs | 48 +++- .../src/storage/erasure_coding/ec_volume.rs | 148 ++++++++++-- seaweed-volume/src/storage/store.rs | 214 +++++++++++++++++- seaweed-volume/src/storage/store_ec_mirror.rs | 35 +++ .../src/storage/store_ec_reconcile.rs | 92 +++++++- 7 files changed, 513 insertions(+), 43 deletions(-) diff --git a/seaweed-volume/src/server/grpc_server.rs b/seaweed-volume/src/server/grpc_server.rs index 4f1fedb21..307aa67c9 100644 --- a/seaweed-volume/src/server/grpc_server.rs +++ b/seaweed-volume/src/server/grpc_server.rs @@ -3565,7 +3565,18 @@ impl VolumeServer for VolumeGrpcService { let file = tokio::fs::File::create(&file_path) .await .map_err(|e| Status::internal(format!("create {}: {}", file_path, e)))?; - drain_copy_stream_to_file(&mut stream, file, &file_path, ".ecx").await?; + let written = drain_copy_stream_to_file(&mut stream, file, &file_path, ".ecx").await?; + // A source that genuinely holds a 0-byte .ecx would leave a stub + // here that no placement or mount decision accepts. Catch it at + // distribute time, as Go does, so the orchestrator can pick another + // source instead of learning about it at mount. + if written == 0 { + let _ = tokio::fs::remove_file(&file_path).await; + return Err(Status::internal(format!( + "VolumeEcShardsCopy volume {}: source .ecx is 0 bytes", + vid + ))); + } } // Copy .ecj file if requested diff --git a/seaweed-volume/src/server/heartbeat.rs b/seaweed-volume/src/server/heartbeat.rs index 0f8363f3a..082475791 100644 --- a/seaweed-volume/src/server/heartbeat.rs +++ b/seaweed-volume/src/server/heartbeat.rs @@ -1849,6 +1849,8 @@ mod tests { let shard_path = format!("{}/ec_metrics_case_27.ec00", dir); std::fs::write(&shard_path, b"ec-shard").unwrap(); + // An EC volume needs its .ecx to mount. + std::fs::write(format!("{}/ec_metrics_case_27.ecx", dir), [0u8; 16]).unwrap(); store.locations[0] .mount_ec_shards(VolumeId(27), "ec_metrics_case", &[0], "") .unwrap(); @@ -1893,6 +1895,8 @@ mod tests { .unwrap(); std::fs::write(format!("{}/expired_heartbeat_ec_31.ec00", dir), b"expired").unwrap(); + // An EC volume needs its .ecx to mount. + std::fs::write(format!("{}/expired_heartbeat_ec_31.ecx", dir), [0u8; 16]).unwrap(); store.locations[0] .mount_ec_shards(VolumeId(31), "expired_heartbeat_ec", &[0], "") .unwrap(); @@ -2194,6 +2198,8 @@ mod tests { let previous = collect_ec_shard_delta_messages(&store); std::fs::write(format!("{}/ec_delta_case_81.ec00", dir), b"delta").unwrap(); + // An EC volume needs its .ecx to mount. + std::fs::write(format!("{}/ec_delta_case_81.ecx", dir), [0u8; 16]).unwrap(); store.locations[0] .mount_ec_shards(VolumeId(81), "ec_delta_case", &[0], "") .unwrap(); diff --git a/seaweed-volume/src/storage/disk_location.rs b/seaweed-volume/src/storage/disk_location.rs index 9a0ab93f5..87dd05b46 100644 --- a/seaweed-volume/src/storage/disk_location.rs +++ b/seaweed-volume/src/storage/disk_location.rs @@ -18,7 +18,7 @@ use crate::storage::erasure_coding::ec_shard::{ DATA_SHARDS_COUNT, ERASURE_CODING_LARGE_BLOCK_SIZE, ERASURE_CODING_SMALL_BLOCK_SIZE, EcVolumeShard, ShardId, }; -use crate::storage::erasure_coding::ec_volume::EcVolume; +use crate::storage::erasure_coding::ec_volume::{EcVolume, is_usable_ecx_file}; use crate::storage::needle_map::NeedleMapKind; use crate::storage::super_block::SUPER_BLOCK_SIZE; use crate::storage::types::*; @@ -779,21 +779,17 @@ impl DiskLocation { /// Mirrors `DiskLocation.HasEcxFileOnDisk` in /// `weed/storage/disk_location_ec.go`. Skips entries that are /// directories so a stray dir named `_.ecx` doesn't - /// register as a present index file. + /// register as a present index file. A 0-byte `.ecx` is a corrupt stub + /// left by a failed EC distribute copy; it must not steer placement + /// toward this disk, so it counts as absent (Go requires `Size() > 0`). pub fn has_ecx_file_on_disk(&self, collection: &str, vid: VolumeId) -> bool { let idx_base = volume_file_name(&self.idx_directory, collection, vid); - let idx_path = format!("{}.ecx", idx_base); - if let Ok(meta) = fs::metadata(&idx_path) - && !meta.is_dir() - { + if is_usable_ecx_file(&format!("{}.ecx", idx_base)) { return true; } if self.idx_directory != self.directory { let data_base = volume_file_name(&self.directory, collection, vid); - let data_path = format!("{}.ecx", data_base); - if let Ok(meta) = fs::metadata(&data_path) - && !meta.is_dir() - { + if is_usable_ecx_file(&format!("{}.ecx", data_base)) { return true; } } @@ -1790,6 +1786,34 @@ mod tests { assert!(loc.find_volume(VolumeId(3)).is_some()); } + /// A 0-byte `.ecx` is the stub a failed EC distribute copy leaves behind. + /// Go's HasEcxFileOnDisk requires Size() > 0 so the stub cannot pin + /// placement to a disk that has no usable index. + #[test] + fn test_has_ecx_file_on_disk_ignores_zero_byte_stub() { + let tmp = TempDir::new().unwrap(); + let data = tmp.path().join("data"); + let idx = tmp.path().join("idx"); + fs::create_dir_all(&data).unwrap(); + fs::create_dir_all(&idx).unwrap(); + let loc = DiskLocation::new( + data.to_str().unwrap(), + idx.to_str().unwrap(), + 10, + DiskType::HardDrive, + MinFreeSpace::Percent(1.0), + Vec::new(), + ) + .unwrap(); + + fs::write(idx.join("pics_7.ecx"), b"").unwrap(); + assert!(!loc.has_ecx_file_on_disk("pics", VolumeId(7))); + + // A real index in the data dir still counts, stub or no stub. + fs::write(data.join("pics_7.ecx"), [0u8; 16]).unwrap(); + assert!(loc.has_ecx_file_on_disk("pics", VolumeId(7))); + } + #[test] fn test_disk_location_delete_collection_removes_ec_volumes() { let tmp = TempDir::new().unwrap(); @@ -1806,6 +1830,8 @@ mod tests { let shard_path = format!("{}/pics_7.ec00", dir); std::fs::write(&shard_path, b"ec-shard").unwrap(); + // An EC volume needs its .ecx to mount. + std::fs::write(format!("{}/pics_7.ecx", dir), [0u8; 16]).unwrap(); loc.mount_ec_shards(VolumeId(7), "pics", &[0], "").unwrap(); assert!(loc.has_ec_volume(VolumeId(7))); @@ -1843,6 +1869,7 @@ mod tests { // mount_ec_shards with source_disk_type="ssd" — simulating the // VolumeEcShardsMount RPC path. std::fs::write(format!("{}/pics_7.ec00", dir), b"ec-shard").unwrap(); + std::fs::write(format!("{}/pics_7.ecx", dir), [0u8; 16]).unwrap(); loc.mount_ec_shards(VolumeId(7), "pics", &[0], "ssd") .unwrap(); { @@ -1942,6 +1969,7 @@ mod tests { // A collection name unique to this test: the gauge is process-global // and sibling tests running in parallel touch other labels. std::fs::write(format!("{}/dupmount_11.ec00", dir), b"shard bytes").unwrap(); + std::fs::write(format!("{}/dupmount_11.ecx", dir), [0u8; 16]).unwrap(); let gauge = crate::metrics::VOLUME_GAUGE.with_label_values(&["dupmount", "ec_shards"]); let before = gauge.get(); diff --git a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs index 745f8cc23..457ad996e 100644 --- a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs +++ b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs @@ -383,8 +383,26 @@ pub fn validate_block_size(block_size: i64) -> io::Result<()> { Ok(()) } +/// Size of the `.ecx` at `path`, or `None` when it is absent or a directory. +/// Mirrors Go's `statEcxSize`. +pub(crate) fn ecx_file_size(path: &str) -> Option { + match std::fs::metadata(path) { + Ok(meta) if !meta.is_dir() => Some(meta.len()), + _ => None, + } +} + +/// Whether `path` is an `.ecx` that can steer placement, ownership or a copy: +/// a regular file with content. A 0-byte `.ecx` is what a failed EC distribute +/// copy leaves behind, and Go treats it as absent at every such decision +/// (`HasEcxFileOnDisk`, `findEcxIdxDirForVolume`, `indexEcxOwners`) so the scan +/// moves on to a sibling disk that may hold a valid index. +pub(crate) fn is_usable_ecx_file(path: &str) -> bool { + ecx_file_size(path).is_some_and(|size| size > 0) +} + impl EcVolume { - /// Create a new EcVolume. Loads .ecx index and .ecj journal if present. + /// Create a new EcVolume. Opens the .ecx index (required) and the .ecj journal. pub fn new( dir: &str, dir_idx: &str, @@ -467,27 +485,62 @@ impl EcVolume { // Open .ecx file (sorted index) in read/write mode for in-place deletion marking. // Matches Go which opens ecx for writing via MarkNeedleDeleted. - let ecx_path = vol.ecx_file_name(); - if std::path::Path::new(&ecx_path).exists() { - let file = open_volume_file(OpenOptions::new().read(true).write(true), &ecx_path)?; - vol.ecx_file_size = file.metadata()?.len() as i64; - vol.ecx_file = Some(file); - } else if dir_idx != dir { - // Fall back to data directory if .ecx was created before -dir.idx was configured - let data_base = crate::storage::volume::volume_file_name(dir, collection, volume_id); - let fallback_ecx = format!("{}.ecx", data_base); - if std::path::Path::new(&fallback_ecx).exists() { - tracing::info!( + // + // Resolve it the way Go's NewEcVolume does: prefer a non-empty copy, + // the one co-located with the shard data first, then the caller's + // index directory — either the shared -dir.idx dir or a sibling disk + // that owns the .ecx when this disk holds only a 0-byte stub left by an + // interrupted copy. A 0-byte .ecx is also a legitimate empty index, so + // it yields only to a non-empty copy elsewhere, never to a mere + // absence. No .ecx at all fails the mount with NotFound (Go wraps + // os.ErrNotExist): an EcVolume without an index would advertise shards + // that no read can ever serve. + let local_ecx = format!( + "{}.ecx", + crate::storage::volume::volume_file_name(dir, collection, volume_id) + ); + let shared_ecx = vol.ecx_file_name(); + let local_size = ecx_file_size(&local_ecx); + let shared_size = if dir_idx != dir { + ecx_file_size(&shared_ecx) + } else { + None + }; + let use_local = match (local_size, shared_size) { + (Some(n), _) if n > 0 => true, + (_, Some(n)) if n > 0 => { + tracing::debug!( volume_id = volume_id.0, - "ecx file not found in idx dir, falling back to data dir" + "ecx not local at {}, using {}", + local_ecx, + shared_ecx ); - let file = - open_volume_file(OpenOptions::new().read(true).write(true), &fallback_ecx)?; - vol.ecx_file_size = file.metadata()?.len() as i64; - vol.ecx_file = Some(file); - vol.ecx_actual_dir = dir.to_string(); + false } - } + // Only 0-byte copies exist: an empty index, local first. + (Some(_), _) => true, + (None, Some(_)) => false, + (None, None) => { + let tried = if dir_idx != dir { + format!("{} (or {})", local_ecx, shared_ecx) + } else { + local_ecx + }; + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("cannot open ec volume index {}: not found", tried), + )); + } + }; + let ecx_path = if use_local { + vol.ecx_actual_dir = dir.to_string(); + local_ecx + } else { + shared_ecx + }; + let file = open_volume_file(OpenOptions::new().read(true).write(true), &ecx_path)?; + vol.ecx_file_size = file.metadata()?.len() as i64; + vol.ecx_file = Some(file); // Open .ecj file (deletion journal) — use ecx_actual_dir for consistency. // Note: Go does NOT replay .ecj into .ecx at volume load (RebuildEcxFile @@ -1864,6 +1917,63 @@ mod tests { use super::*; use tempfile::TempDir; + /// Go's NewEcVolume fails with os.ErrNotExist when neither directory has an + /// `.ecx`. Mounting anyway advertises shards every read then fails on with + /// "ecx file not open", and zeroes the size `add_shard`'s 0-byte guard needs. + #[test] + fn test_new_without_ecx_is_not_found() { + let data = TempDir::new().unwrap(); + let idx = TempDir::new().unwrap(); + let (dir, dir_idx) = (data.path().to_str().unwrap(), idx.path().to_str().unwrap()); + std::fs::write(format!("{}/7.ec00", dir), b"shard").unwrap(); + + for idx_dir in [dir, dir_idx] { + let err = EcVolume::new(dir, idx_dir, "", VolumeId(7)) + .err() + .expect("an EC volume without an .ecx must not mount"); + assert_eq!(err.kind(), io::ErrorKind::NotFound, "{}", err); + } + } + + /// A 0-byte `.ecx` stub left by an interrupted copy yields to a non-empty + /// copy in the other directory, whichever side the stub is on. + #[test] + fn test_new_prefers_non_empty_ecx_over_zero_byte_stub() { + for stub_in_idx_dir in [true, false] { + let data = TempDir::new().unwrap(); + let idx = TempDir::new().unwrap(); + let (dir, dir_idx) = (data.path().to_str().unwrap(), idx.path().to_str().unwrap()); + let (stub_dir, valid_dir) = if stub_in_idx_dir { + (dir_idx, dir) + } else { + (dir, dir_idx) + }; + std::fs::write(format!("{}/7.ecx", stub_dir), b"").unwrap(); + std::fs::write( + format!("{}/7.ecx", valid_dir), + vec![0u8; NEEDLE_MAP_ENTRY_SIZE], + ) + .unwrap(); + + let vol = EcVolume::new(dir, dir_idx, "", VolumeId(7)).unwrap(); + assert_eq!(vol.ecx_actual_dir(), valid_dir); + assert_eq!(vol.ecx_file_size, NEEDLE_MAP_ENTRY_SIZE as i64); + } + } + + /// With no other copy a 0-byte `.ecx` is a legitimate empty index (a volume + /// whose needles were all deleted before encoding) and still mounts, as in Go. + #[test] + fn test_new_accepts_lone_zero_byte_ecx_as_empty_index() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + std::fs::write(format!("{}/7.ecx", dir), b"").unwrap(); + + let vol = EcVolume::new(dir, dir, "", VolumeId(7)).unwrap(); + assert_eq!(vol.ecx_file_size, 0); + assert!(!is_usable_ecx_file(&vol.ecx_file_name())); + } + /// `destroy()` must remove co-located `.ecsum` sidecars (Go Destroy parity). /// Without this, `collection.delete` leaves orphaned bitrot files that /// inflate EC-health scanners after the shards are gone. diff --git a/seaweed-volume/src/storage/store.rs b/seaweed-volume/src/storage/store.rs index ee5edd638..fc5411957 100644 --- a/seaweed-volume/src/storage/store.rs +++ b/seaweed-volume/src/storage/store.rs @@ -13,7 +13,7 @@ use crate::config::MinFreeSpace; use crate::pb::master_pb; use crate::storage::disk_location::DiskLocation; use crate::storage::erasure_coding::ec_shard::{EcVolumeShard, MAX_SHARD_COUNT, ShardId}; -use crate::storage::erasure_coding::ec_volume::EcVolume; +use crate::storage::erasure_coding::ec_volume::{EcVolume, is_usable_ecx_file}; use crate::storage::needle::needle::Needle; use crate::storage::needle_map::NeedleMapKind; use crate::storage::super_block::ReplicaPlacement; @@ -905,18 +905,88 @@ impl Store { shard_id: ShardId, source_disk_type: &str, ) -> Result<(), VolumeError> { + // The .ecx may live on a different disk than the shard being mounted + // (ec.balance / ec.rebuild can spread shards across sibling disks), so + // look up its owner once and point EcVolume::new at the directory that + // really has it — Go's MountEcShards does the same. Without an index + // anywhere the mount fails instead of advertising an unreadable shard. + let ecx_idx_dir = self.find_ecx_idx_dir_for_volume(collection, vid); + // Keep going past a disk that cannot mount the shard: an interrupted + // move can leave an unusable copy on one disk and a good one on the + // next. Like Go, a NotFound just means "not this disk"; anything else + // is collected so an all-disks-fail error names every disk tried. + let mut failures: Vec<(String, VolumeError)> = Vec::new(); for loc in &mut self.locations { // Check if the shard file exists on this location let shard = EcVolumeShard::new(&loc.directory, collection, vid, shard_id); - if std::path::Path::new(&shard.file_name()).exists() { - loc.mount_ec_shards(vid, collection, &[shard_id], source_disk_type)?; - return Ok(()); + if !std::path::Path::new(&shard.file_name()).exists() { + continue; + } + // If this disk owns the .ecx its own idx dir is the right answer; + // only a disk without a usable .ecx is pointed at the owner's. + let idx_dir = match &ecx_idx_dir { + Some(owner_dir) + if loc.idx_directory != *owner_dir + && loc.directory != *owner_dir + && !loc.has_ecx_file_on_disk(collection, vid) => + { + owner_dir.clone() + } + _ => loc.idx_directory.clone(), + }; + match loc.mount_ec_shards_with_idx_dir( + vid, + collection, + &[shard_id], + &idx_dir, + source_disk_type, + ) { + Ok(()) => return Ok(()), + Err(VolumeError::Io(e)) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => failures.push((loc.directory.clone(), e)), } } - Err(VolumeError::Io(io::Error::new( - io::ErrorKind::NotFound, - format!("MountEcShards {}.{} not found on disk", vid, shard_id), - ))) + if failures.is_empty() { + let what = if ecx_idx_dir.is_none() { + ": no .ecx index found on any local disk" + } else { + " not found on disk" + }; + return Err(VolumeError::Io(io::Error::new( + io::ErrorKind::NotFound, + format!("MountEcShards {}.{}{}", vid, shard_id, what), + ))); + } + let tried = failures + .iter() + .map(|(dir, e)| format!("{}: {}", dir, e)) + .collect::>() + .join("; "); + Err(VolumeError::Io(io::Error::other(format!( + "MountEcShards {}.{} load failures: {}", + vid, shard_id, tried + )))) + } + + /// The directory holding a usable `.ecx` for (collection, vid) on any local + /// disk, index directory before data directory. A 0-byte `.ecx` is a stub + /// from a failed copy and counts as absent, so the scan continues to a + /// sibling disk. Mirrors Go's `Store.findEcxIdxDirForVolume`. + fn find_ecx_idx_dir_for_volume(&self, collection: &str, vid: VolumeId) -> Option { + let mut seen = std::collections::HashSet::new(); + for loc in &self.locations { + for scan in [&loc.idx_directory, &loc.directory] { + // A shared -dir.idx is only stat'd once per call. + if scan.is_empty() || !seen.insert(scan.clone()) { + continue; + } + let base = crate::storage::volume::volume_file_name(scan, collection, vid); + if is_usable_ecx_file(&format!("{}.ecx", base)) { + return Some(scan.clone()); + } + } + } + None } /// Unmount EC shards for a volume (batch). @@ -1203,7 +1273,9 @@ impl Store { for (i, loc) in self.locations.iter().enumerate() { let base = crate::storage::volume::volume_file_name(&loc.directory, collection, vid); let ecx_path = format!("{}.ecx", base); - if std::path::Path::new(&ecx_path).exists() { + // A 0-byte .ecx is a failed-copy stub, not an index (Go requires + // Size() > 0); keep looking for a disk with a real one. + if is_usable_ecx_file(&ecx_path) { return Some(i); } } @@ -2263,6 +2335,8 @@ mod tests { let mut store = make_test_store(&[dir]); std::fs::write(format!("{}/expired_ec_case_9.ec00", dir), b"expired").unwrap(); + // An EC volume needs its .ecx to mount. + std::fs::write(format!("{}/expired_ec_case_9.ecx", dir), [0u8; 16]).unwrap(); store.locations[0] .mount_ec_shards(VolumeId(9), "expired_ec_case", &[0], "") .unwrap(); @@ -2553,6 +2627,14 @@ mod tests { b"shard data", ) .unwrap(); + std::fs::write( + format!( + "{}/{}_{}.ecx", + store.locations[1].directory, collection, vid.0 + ), + vec![0u8; 20], + ) + .unwrap(); store.locations[1] .mount_ec_shards(vid, collection, &[0], "") .unwrap(); @@ -2628,6 +2710,14 @@ mod tests { b"shard data", ) .unwrap(); + std::fs::write( + format!( + "{}/{}_{}.ecx", + store.locations[1].directory, collection, vid.0 + ), + vec![0u8; 20], + ) + .unwrap(); store.locations[1] .mount_ec_shards(vid, collection, &[0], "") .unwrap(); @@ -2661,12 +2751,14 @@ mod tests { let base0 = volume_file_name(&store.locations[0].directory, collection, vid); std::fs::write(format!("{}.ec00", base0), b"x").unwrap(); std::fs::write(format!("{}.ec01", base0), b"x").unwrap(); + std::fs::write(format!("{}.ecx", base0), vec![0u8; 20]).unwrap(); store.locations[0] .mount_ec_shards(vid, collection, &[0, 1], "") .unwrap(); let base1 = volume_file_name(&store.locations[1].directory, collection, vid); std::fs::write(format!("{}.ec02", base1), b"x").unwrap(); + std::fs::write(format!("{}.ecx", base1), vec![0u8; 20]).unwrap(); store.locations[1] .mount_ec_shards(vid, collection, &[2], "") .unwrap(); @@ -2706,6 +2798,7 @@ mod tests { let base = volume_file_name(&store.locations[0].directory, collection, vid); std::fs::write(format!("{}.ec00", base), b"x").unwrap(); + std::fs::write(format!("{}.ecx", base), vec![0u8; 20]).unwrap(); store.locations[0] .mount_ec_shards(vid, collection, &[0], "") .unwrap(); @@ -2717,6 +2810,7 @@ mod tests { for shard_id in &filler_shards { std::fs::write(format!("{}.ec{:02}", filler_base, shard_id), b"x").unwrap(); } + std::fs::write(format!("{}.ecx", filler_base), vec![0u8; 20]).unwrap(); store.locations[0] .mount_ec_shards(filler, collection, &filler_shards, "") .unwrap(); @@ -2734,6 +2828,106 @@ mod tests { ); } + /// Per-shard `VolumeEcShardsMount` with no `.ecx` on any local disk must + /// fail (Go: "no .ecx index found on any local disk") rather than register + /// a volume whose every read dies with "ecx file not open". + #[test] + fn test_mount_ec_shard_without_ecx_fails_and_advertises_nothing() { + let (mut store, _tmp) = make_ec_target_test_store(2); + let collection = "grafana-loki"; + let vid = VolumeId(12121); + let base = volume_file_name(&store.locations[1].directory, collection, vid); + std::fs::write(format!("{}.ec00", base), b"x").unwrap(); + + let err = store + .mount_ec_shard(vid, collection, 0, "") + .expect_err("a shard without an index must not mount"); + assert!( + matches!(&err, VolumeError::Io(e) if e.kind() == io::ErrorKind::NotFound), + "got {:?}", + err + ); + assert!(store.find_ec_volume(vid).is_none()); + } + + /// An interrupted move can leave a 0-byte shard on one disk and the good + /// copy on the next. The first disk's failure must not end the scan (Go's + /// MountEcShards keeps going), and nothing of the failed attempt may stay + /// registered. + #[test] + fn test_mount_ec_shard_continues_past_a_disk_that_cannot_mount() { + let (mut store, _tmp) = make_ec_target_test_store(2); + let collection = "grafana-loki"; + let vid = VolumeId(13132); + for loc in &store.locations { + let base = volume_file_name(&loc.directory, collection, vid); + std::fs::write(format!("{}.ecx", base), vec![0u8; 20]).unwrap(); + } + let bad = volume_file_name(&store.locations[0].directory, collection, vid); + std::fs::write(format!("{}.ec00", bad), b"").unwrap(); + let good = volume_file_name(&store.locations[1].directory, collection, vid); + std::fs::write(format!("{}.ec00", good), b"x").unwrap(); + + store.mount_ec_shard(vid, collection, 0, "").unwrap(); + + assert!(store.locations[0].find_ec_volume(vid).is_none()); + assert!(store.locations[1].find_ec_volume(vid).unwrap().has_shard(0)); + } + + /// When every disk holding the shard fails, the error names each of them. + #[test] + fn test_mount_ec_shard_reports_every_failing_disk() { + let (mut store, _tmp) = make_ec_target_test_store(2); + let collection = "grafana-loki"; + let vid = VolumeId(13133); + for loc in &store.locations { + let base = volume_file_name(&loc.directory, collection, vid); + std::fs::write(format!("{}.ecx", base), vec![0u8; 20]).unwrap(); + std::fs::write(format!("{}.ec00", base), b"").unwrap(); + } + + let err = store + .mount_ec_shard(vid, collection, 0, "") + .unwrap_err() + .to_string(); + + for loc in &store.locations { + assert!( + err.contains(&loc.directory), + "{} missing from: {}", + loc.directory, + err + ); + assert!(loc.find_ec_volume(vid).is_none()); + } + } + + /// The `.ecx` may sit on a sibling disk of the one holding the shard; the + /// per-shard mount routes EcVolume::new at the owner's directory, skipping + /// a 0-byte stub on the way (Go's findEcxIdxDirForVolume). + #[test] + fn test_mount_ec_shard_uses_valid_ecx_on_sibling_disk() { + let (mut store, _tmp) = make_ec_target_test_store(3); + let collection = "grafana-loki"; + let vid = VolumeId(13131); + let stub = volume_file_name(&store.locations[0].directory, collection, vid); + std::fs::write(format!("{}.ecx", stub), b"").unwrap(); + let owner_dir = store.locations[1].directory.clone(); + let owner = volume_file_name(&owner_dir, collection, vid); + std::fs::write(format!("{}.ecx", owner), vec![0u8; 20]).unwrap(); + let shard = volume_file_name(&store.locations[2].directory, collection, vid); + std::fs::write(format!("{}.ec00", shard), b"x").unwrap(); + + // The stub is not a location for the batch path either. + assert_eq!(store.find_ec_location(vid, collection), Some(1)); + + store.mount_ec_shard(vid, collection, 0, "").unwrap(); + let ec_vol = store.locations[2] + .find_ec_volume(vid) + .expect("mounted on the shard's disk"); + assert_eq!(ec_vol.ecx_actual_dir(), owner_dir); + } + /// Mixed-owner batch contract: a batch whose requested shards are /// already owned by different disks reports every owner, so /// `volume_ec_shards_copy` can refuse it rather than rank the owners @@ -2747,11 +2941,13 @@ mod tests { let base0 = volume_file_name(&store.locations[0].directory, collection, vid); std::fs::write(format!("{}.ec00", base0), b"x").unwrap(); std::fs::write(format!("{}.ec01", base0), b"x").unwrap(); + std::fs::write(format!("{}.ecx", base0), vec![0u8; 20]).unwrap(); store.locations[0] .mount_ec_shards(vid, collection, &[0, 1], "") .unwrap(); let base1 = volume_file_name(&store.locations[1].directory, collection, vid); std::fs::write(format!("{}.ec02", base1), b"x").unwrap(); + std::fs::write(format!("{}.ecx", base1), vec![0u8; 20]).unwrap(); store.locations[1] .mount_ec_shards(vid, collection, &[2], "") .unwrap(); diff --git a/seaweed-volume/src/storage/store_ec_mirror.rs b/seaweed-volume/src/storage/store_ec_mirror.rs index 1e53e4a34..95ae9df89 100644 --- a/seaweed-volume/src/storage/store_ec_mirror.rs +++ b/seaweed-volume/src/storage/store_ec_mirror.rs @@ -131,6 +131,12 @@ impl Store { let Some(base) = name.strip_suffix(".ecx") else { continue; }; + // A 0-byte .ecx is a corrupt stub from a failed copy, not a + // credible owner — skip it so the scan keeps looking for a + // real index on a sibling disk (Go's indexEcxOwners). + if !ent.metadata().is_ok_and(|m| m.len() > 0) { + continue; + } let Some((collection, vid)) = parse_collection_volume_id_pub(base) else { continue; }; @@ -417,4 +423,33 @@ mod tests { let post = fs::read(dir0.join(format!("{}_{}.ecx", collection, vid))).unwrap(); assert_eq!(post, ecx_local, "mirror overwrote dir0's existing .ecx"); } + + /// The mirror shares Go's indexEcxOwners, which skips a 0-byte `.ecx`: + /// a stub must not be chosen as the source to mirror from. + #[test] + fn mirror_owner_index_skips_zero_byte_ecx() { + let tmp = TempDir::new().unwrap(); + let dir0 = tmp.path().join("data0"); + let dir1 = tmp.path().join("data1"); + fs::create_dir_all(&dir0).unwrap(); + fs::create_dir_all(&dir1).unwrap(); + + let collection = "video-recordings"; + let vid = 4123u32; + plant_ecx(&dir0, collection, vid, b""); + plant_ecx(&dir1, collection, vid, &[0xA1u8; 20]); + + let mut store = Store::new(NeedleMapKind::InMemory); + add_loc(&mut store, &dir0); + add_loc(&mut store, &dir1); + + let owners = store.index_ecx_owners_for_mirror(); + let owner = owners + .get(&EcKey { + collection: collection.to_string(), + vid: VolumeId(vid), + }) + .expect("the valid .ecx on disk 1 must be indexed"); + assert_eq!(owner.location, 1); + } } diff --git a/seaweed-volume/src/storage/store_ec_reconcile.rs b/seaweed-volume/src/storage/store_ec_reconcile.rs index f5253832d..38ac80a1c 100644 --- a/seaweed-volume/src/storage/store_ec_reconcile.rs +++ b/seaweed-volume/src/storage/store_ec_reconcile.rs @@ -37,6 +37,7 @@ pub(crate) struct EcVolumeMissingIndex { pub data_dir: String, } +#[cfg(test)] pub(crate) fn ec_local_ecx_path(dir: &str, collection: &str, vid: VolumeId) -> String { if collection.is_empty() { format!("{}/{}.ecx", dir, vid.0) @@ -117,10 +118,9 @@ impl Store { ); continue; }; - let local_ecx = ec_local_ecx_path(&loc.idx_directory, &key.collection, key.vid); - let local_ecx_in_data = ec_local_ecx_path(&loc.directory, &key.collection, key.vid); - let use_local_idx = std::path::Path::new(&local_ecx).exists() - || std::path::Path::new(&local_ecx_in_data).exists(); + // A 0-byte local stub is not a mirrored index (Go gates this fast + // path on HasEcxFileOnDisk); mount against the owner instead. + let use_local_idx = loc.has_ecx_file_on_disk(&key.collection, key.vid); if !use_local_idx && owner.location == loc_idx && owner.idx_dir == loc.idx_directory { @@ -422,6 +422,12 @@ impl Store { let Some(base) = name.strip_suffix(".ecx") else { continue; }; + // A 0-byte .ecx is a corrupt stub from a failed copy, not a + // credible owner — skip it so the scan keeps looking for a + // real index on a sibling disk (Go's indexEcxOwners). + if !ent.metadata().is_ok_and(|m| m.len() > 0) { + continue; + } let Some((collection, vid)) = parse_collection_volume_id_pub(base) else { continue; }; @@ -639,6 +645,34 @@ mod tests { .unwrap(); } + /// A 0-byte `.ecx` is not a credible owner (Go's indexEcxOwners skips + /// it): picking the stub would hide the valid index on the sibling disk. + #[test] + fn test_index_ecx_owners_skips_zero_byte_stub() { + let (store, _tmp) = make_test_store(2, None); + let d0 = store.locations[0].directory.clone(); + let d1 = store.locations[1].directory.clone(); + std::fs::write(ec_local_ecx_path(&d0, "pics", VolumeId(7)), b"").unwrap(); + write_index_files(&d1, "pics", 7, 10, 4); + + let owners = store.index_ecx_owners(); + let owner = owners + .get(&EcKey { + collection: "pics".to_string(), + vid: VolumeId(7), + }) + .expect("the valid .ecx on disk 1 must be indexed"); + assert_eq!(owner.location, 1); + assert_eq!(owner.idx_dir, d1); + + // A stub with no real index anywhere owns nothing. + std::fs::write(ec_local_ecx_path(&d0, "pics", VolumeId(8)), b"").unwrap(); + assert!(!store.index_ecx_owners().contains_key(&EcKey { + collection: "pics".to_string(), + vid: VolumeId(8), + })); + } + /// An empty `.dat` (<= a superblock, i.e. zero needles) for an EC volume /// is a leftover stub from the pre-fix loader. It must be swept on startup, /// not loaded as a phantom empty volume. With the same vid's stub on two @@ -971,6 +1005,56 @@ mod tests { ); } + /// dir0 holds orphan shards next to a 0-byte `.ecx` stub from a failed + /// copy; the real index is on dir1. The stub must not count as a + /// locally-mirrored index (Go gates that fast path on HasEcxFileOnDisk), + /// or the shards get registered against an empty index. + #[test] + fn test_reconcile_ignores_zero_byte_local_ecx_stub() { + let tmp = TempDir::new().unwrap(); + let dir0 = tmp.path().join("data0"); + let dir1 = tmp.path().join("data1"); + std::fs::create_dir_all(&dir0).unwrap(); + std::fs::create_dir_all(&dir1).unwrap(); + + let collection = "grafana-loki"; + let vid = 1094u32; + + write_shard(dir0.to_str().unwrap(), collection, vid, 0); + write_shard(dir1.to_str().unwrap(), collection, vid, 1); + write_index_files(dir1.to_str().unwrap(), collection, vid, 10, 4); + + let mut store = Store::new(NeedleMapKind::InMemory); + for dir in [&dir0, &dir1] { + store + .add_location( + dir.to_str().unwrap(), + dir.to_str().unwrap(), + 100, + DiskType::HardDrive, + MinFreeSpace::Percent(0.0), + Vec::new(), + ) + .unwrap(); + } + // Plant the stub after the startup scan so only the reconcile decision + // is under test, then drop dir0's mount and reconcile again. + store.locations[0].remove_ec_volume(VolumeId(vid)); + std::fs::write( + ec_local_ecx_path(dir0.to_str().unwrap(), collection, VolumeId(vid)), + b"", + ) + .unwrap(); + + store.reconcile_ec_shards_across_disks(); + + let ev0 = store.locations[0] + .find_ec_volume(VolumeId(vid)) + .expect("dir0's shard must be mounted against the owner's index"); + assert!(ev0.has_shard(0)); + assert_eq!(ev0.ecx_actual_dir(), dir1.to_str().unwrap()); + } + /// PR 9244 review case: idx_directory is configured but the /// owner's .ecx / .ecj / .vif live in the owner's data dir /// (the legacy "written before -dir.idx was set" layout). The