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