From ed81d2f6b8693b3ed985cfcfbf37a313fdb81c37 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 9 Jul 2026 03:12:48 +0800 Subject: [PATCH] test(ecstore): complete EC validation coverage gate * test(ecstore): complete EC validation coverage gate * test(ecstore): stabilize validation suite after rebase * test(ecstore): fix rio-v2 clippy lint --- crates/ecstore/src/disk/local.rs | 768 +++++++++++++++++- .../src/ecstore_validation_blackbox.rs | 434 ++++++++++ crates/ecstore/src/erasure/codec/bridge.rs | 101 +++ crates/ecstore/src/erasure/codec/workspace.rs | 13 + crates/ecstore/src/erasure/coding/bitrot.rs | 169 +++- crates/ecstore/src/erasure/coding/decode.rs | 439 ++++++++-- .../src/erasure/coding/decode_reader.rs | 614 +++++++++++++- crates/ecstore/src/erasure/coding/encode.rs | 234 ++++-- crates/ecstore/src/erasure/coding/erasure.rs | 243 ++++-- crates/ecstore/src/erasure/coding/heal.rs | 107 ++- crates/ecstore/src/lib.rs | 3 + .../src/set_disk/core/io_primitives.rs | 567 ++++++++++++- crates/ecstore/src/set_disk/metadata.rs | 98 +++ crates/ecstore/src/set_disk/mod.rs | 757 ++++++++++++++++- crates/ecstore/src/set_disk/read.rs | 594 ++++++++++++++ crates/ecstore/src/set_disk/shard_source.rs | 12 + crates/ecstore/src/store/bucket.rs | 13 + scripts/run_ecstore_validation_suite.sh | 14 +- 18 files changed, 4916 insertions(+), 264 deletions(-) create mode 100644 crates/ecstore/src/ecstore_validation_blackbox.rs diff --git a/crates/ecstore/src/disk/local.rs b/crates/ecstore/src/disk/local.rs index 1f58cdff6..d46f58e17 100644 --- a/crates/ecstore/src/disk/local.rs +++ b/crates/ecstore/src/disk/local.rs @@ -2755,7 +2755,8 @@ impl LocalDisk { return false; } - if cfg!(target_os = "windows") { + #[cfg(target_os = "windows")] + { // Windows volume names must not include reserved characters. // This regular expression matches disallowed characters. if volname.contains('|') @@ -2769,8 +2770,6 @@ impl LocalDisk { { return false; } - } else { - // Non-Windows systems may require additional validation rules. } true @@ -5946,10 +5945,11 @@ async fn get_disk_info(drive_path: PathBuf) -> Result<(rustfs_utils::os::DiskInf #[cfg(test)] mod test { use super::*; + use rustfs_filemeta::ErasureInfo; use std::io; use std::pin::Pin; use std::task::{Context, Poll}; - use tokio::io::{AsyncReadExt, AsyncWrite, ReadBuf}; + use tokio::io::{AsyncReadExt, AsyncWrite, AsyncWriteExt, ReadBuf}; fn test_file_info(name: &str, version_id: Uuid, data_dir: Option, data: Option) -> FileInfo { let size = data @@ -6275,6 +6275,381 @@ mod test { } } + #[tokio::test] + async fn startup_cleanup_barrier_and_tmp_trash_cleanup_cover_noop_and_delete_paths() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + disk.startup_cleanup_ready.store(0, Ordering::Release); + let ready = Arc::clone(&disk.startup_cleanup_ready); + let notify = Arc::clone(&disk.startup_cleanup_notify); + tokio::spawn(async move { + ready.store(1, Ordering::Release); + notify.notify_waiters(); + }); + disk.wait_for_startup_cleanup().await; + assert_eq!(disk.startup_cleanup_ready.load(Ordering::Acquire), 1); + + LocalDisk::cleanup_stale_tmp_objects_with_expiry(dir.path().join("missing-root"), Duration::ZERO) + .await + .expect("missing tmp path should be a cleanup no-op"); + LocalDisk::cleanup_deleted_objects(dir.path().join("missing-root")) + .await + .expect("missing trash path should be a cleanup no-op"); + + let tmp_root = dir.path().join(RUSTFS_META_TMP_BUCKET); + let stale_dir = tmp_root.join("stale-upload"); + let live_file = tmp_root.join("part-file"); + let trash_root = dir.path().join(RUSTFS_META_TMP_DELETED_BUCKET); + fs::create_dir_all(&stale_dir).await.expect("stale dir should be created"); + fs::write(&live_file, b"not-a-dir").await.expect("tmp file should be created"); + fs::create_dir_all(&trash_root).await.expect("trash dir should be created"); + tokio::time::sleep(Duration::from_millis(1)).await; + + LocalDisk::cleanup_stale_tmp_objects_with_expiry(dir.path().to_path_buf(), Duration::ZERO) + .await + .expect("stale tmp directory should move to trash"); + assert!(!stale_dir.exists(), "stale tmp directory should be moved away"); + assert!(live_file.exists(), "plain tmp files should be ignored by stale dir cleanup"); + + fs::write(trash_root.join("trash-file"), b"delete me") + .await + .expect("trash file should be created"); + fs::create_dir_all(trash_root.join("trash-dir")) + .await + .expect("trash dir should be created"); + LocalDisk::cleanup_deleted_objects(dir.path().to_path_buf()) + .await + .expect("trash cleanup should remove files and directories"); + assert!( + fs::read_dir(&trash_root) + .await + .expect("trash root should exist") + .next_entry() + .await + .unwrap() + .is_none() + ); + } + + #[tokio::test] + async fn path_cache_covers_absolute_relative_batch_hit_miss_and_eviction() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let absolute = disk + .resolve_abs_path(dir.path().join("absolute-object")) + .expect("absolute path should resolve"); + assert!(absolute.ends_with("absolute-object")); + + { + let mut cache = disk.path_cache.write(); + for index in 0..4096 { + cache.insert(format!("cached-{index}"), dir.path().join(format!("cached-{index}"))); + } + } + let relative = disk.resolve_abs_path("bucket/object").expect("relative path should resolve"); + assert!(relative.ends_with("bucket/object")); + assert!( + disk.path_cache.read().len() < 4097, + "cache eviction should run before inserting a new path" + ); + + let requests = vec![ + ("bucket".to_string(), "a".to_string()), + ("bucket".to_string(), "b".to_string()), + ]; + let first = disk + .get_object_paths_batch(&requests) + .expect("batch path resolution should handle cache misses"); + assert_eq!(first.len(), 2); + assert!(first[0].ends_with("bucket/a")); + assert!(first[1].ends_with("bucket/b")); + + let second = disk + .get_object_paths_batch(&requests) + .expect("batch path resolution should reuse cache hits"); + assert_eq!(second, first); + } + + #[tokio::test] + async fn open_file_read_only_returns_existing_payload_without_parent_creation() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + let path = dir.path().join("bucket/object/part.1"); + fs::create_dir_all(path.parent().expect("test file should have a parent")) + .await + .expect("parent directory should be created"); + fs::write(&path, b"read-only-payload") + .await + .expect("test file should be written"); + + let mut file = disk.open_file_read_only(&path).await.expect("read-only file should open"); + let mut payload = Vec::new(); + file.read_to_end(&mut payload).await.expect("read-only file should read"); + + assert_eq!(payload, b"read-only-payload"); + } + + #[tokio::test] + async fn write_metadata_replaces_corrupt_existing_xl_meta_without_losing_new_version() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + let bucket = "metadata-rewrite-bucket"; + let object = "nested/object"; + ensure_test_volume(&disk, bucket).await; + + let object_dir = dir.path().join(bucket).join(object); + fs::create_dir_all(&object_dir).await.expect("object dir should be created"); + fs::write(object_dir.join(STORAGE_FORMAT_FILE), b"not-valid-xl-meta") + .await + .expect("corrupt metadata should be installed"); + + let version_id = Uuid::new_v4(); + let mut fi = test_file_info(object, version_id, Some(Uuid::new_v4()), Some(Bytes::from_static(b"restored"))); + fi.fresh = false; + disk.write_metadata(bucket, bucket, object, fi) + .await + .expect("new metadata write should replace corrupt old metadata"); + + let raw = disk + .read_all(bucket, &format!("{object}/{STORAGE_FORMAT_FILE}")) + .await + .expect("rewritten metadata should be readable"); + let restored = FileMeta::load(&raw) + .expect("rewritten metadata should decode") + .into_fileinfo(bucket, object, "", true, false, true) + .expect("rewritten metadata should expose the new version"); + + assert_eq!(restored.version_id, Some(version_id)); + assert_eq!(restored.name, object); + } + + fn test_check_parts_file_info(data_dir: Uuid) -> FileInfo { + FileInfo { + name: "dir/object".to_string(), + data_dir: Some(data_dir), + parts: (1..=4) + .map(|number| ObjectPartInfo { + number, + size: 5, + actual_size: 5, + ..Default::default() + }) + .collect(), + erasure: ErasureInfo { + data_blocks: 2, + parity_blocks: 2, + block_size: 4, + distribution: vec![1, 2, 3, 4], + ..Default::default() + }, + ..Default::default() + } + } + + #[tokio::test] + async fn test_check_parts_classifies_part_and_volume_failures() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let bucket = "bucket"; + let object = "dir/object"; + let data_dir = Uuid::parse_str("01010101-0101-0101-0101-010101010101").expect("data dir should parse"); + let fi = test_check_parts_file_info(data_dir); + + ensure_test_volume(&disk, bucket).await; + let part_dir = dir.path().join(bucket).join(object).join(data_dir.to_string()); + fs::create_dir_all(&part_dir).await.expect("part dir should be created"); + fs::write(part_dir.join("part.1"), vec![1; 4]) + .await + .expect("valid part should be written"); + fs::write(part_dir.join("part.2"), vec![2; 3]) + .await + .expect("short part should be written"); + fs::create_dir_all(part_dir.join("part.3")) + .await + .expect("directory part marker should be created"); + + let resp = disk + .check_parts(bucket, object, &fi) + .await + .expect("check_parts should return per-part status"); + assert_eq!( + resp.results, + vec![ + CHECK_PART_SUCCESS, + CHECK_PART_FILE_CORRUPT, + CHECK_PART_FILE_NOT_FOUND, + CHECK_PART_FILE_NOT_FOUND, + ], + "valid, short, directory, and missing parts must be classified distinctly" + ); + + let missing_volume_resp = disk + .check_parts("missing-bucket", object, &fi) + .await + .expect("missing volume should be reported per part"); + assert_eq!( + missing_volume_resp.results, + vec![CHECK_PART_VOLUME_NOT_FOUND; fi.parts.len()], + "missing volume must not be reported as recoverable missing shards" + ); + } + + #[tokio::test] + async fn test_read_parts_reports_bad_metadata_and_missing_data_part() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let bucket = "bucket"; + ensure_test_volume(&disk, bucket).await; + + let valid_part = ObjectPartInfo { + etag: "etag-1".to_string(), + number: 1, + size: 5, + actual_size: 5, + ..Default::default() + }; + disk.write_all(bucket, "upload/part.1", Bytes::from_static(b"data-1")) + .await + .expect("part data should be written"); + disk.write_all( + bucket, + "upload/part.1.meta", + Bytes::from(valid_part.marshal_msg().expect("part metadata should encode")), + ) + .await + .expect("part metadata should be written"); + disk.write_all(bucket, "upload/part.2", Bytes::from_static(b"data-2")) + .await + .expect("second part data should be written"); + disk.write_all(bucket, "upload/part.2.meta", Bytes::from_static(b"not-msgpack")) + .await + .expect("bad part metadata should be written"); + disk.write_all(bucket, "upload/part.3.meta", Bytes::from_static(b"orphan-meta")) + .await + .expect("orphan metadata should be written"); + + let parts = disk + .read_parts( + bucket, + &[ + "upload/part.1.meta".to_string(), + "upload/part.2.meta".to_string(), + "upload/part.3.meta".to_string(), + ], + ) + .await + .expect("read_parts should return per-part status"); + + assert_eq!(parts[0], valid_part); + assert_eq!(parts[1].number, 2); + assert!( + parts[1].error.is_some(), + "bad metadata must be surfaced as a per-part error instead of a decoded part" + ); + assert_eq!(parts[2].number, 3); + assert!(parts[2].error.is_some(), "missing data part must be surfaced as a per-part error"); + } + + #[tokio::test] + async fn test_rename_part_rejects_type_mismatch_without_touching_source() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let tmp_volume = "tmp"; + let bucket = "bucket"; + ensure_test_volume(&disk, tmp_volume).await; + ensure_test_volume(&disk, bucket).await; + + let payload = Bytes::from_static(b"part payload"); + disk.write_all(tmp_volume, "upload/part.1", payload.clone()) + .await + .expect("source part should be written"); + + let result = disk + .rename_part(tmp_volume, "upload/part.1", bucket, "object/part.1/", Bytes::from_static(b"metadata")) + .await; + assert!( + matches!(result, Err(DiskError::FileAccessDenied)), + "file-to-directory rename_part mismatch must be rejected, got {result:?}" + ); + assert_eq!( + disk.read_all(tmp_volume, "upload/part.1") + .await + .expect("source part must remain after rejected rename"), + payload + ); + assert!( + matches!(disk.read_all(bucket, "object/part.1.meta").await, Err(DiskError::FileNotFound)), + "rejected rename_part must not write destination metadata" + ); + } + + #[tokio::test] + async fn test_rename_part_commits_data_and_metadata_then_removes_source() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let tmp_volume = "tmp"; + let bucket = "bucket"; + ensure_test_volume(&disk, tmp_volume).await; + ensure_test_volume(&disk, bucket).await; + + let payload = Bytes::from_static(b"part payload"); + let meta = Bytes::from_static(b"part metadata"); + disk.write_all(tmp_volume, "upload/part.1", payload.clone()) + .await + .expect("source part should be written"); + + disk.rename_part(tmp_volume, "upload/part.1", bucket, "object/part.1", meta.clone()) + .await + .expect("rename_part should commit part"); + + assert_eq!( + disk.read_all(bucket, "object/part.1") + .await + .expect("destination part should be readable"), + payload + ); + assert_eq!( + disk.read_all(bucket, "object/part.1.meta") + .await + .expect("destination metadata should be readable"), + meta + ); + assert!( + matches!(disk.read_all(tmp_volume, "upload/part.1").await, Err(DiskError::FileNotFound)), + "source part must be removed after a successful commit" + ); + } + struct BlockingScanWriter { entered_tx: Option>, } @@ -6296,6 +6671,25 @@ mod test { } } + #[tokio::test] + async fn blocking_scan_writer_keeps_flush_and_shutdown_pending() { + let mut flush_writer = BlockingScanWriter { entered_tx: None }; + assert!( + tokio::time::timeout(Duration::from_millis(10), flush_writer.flush()) + .await + .is_err(), + "blocking scan writer flush should stay pending" + ); + + let mut shutdown_writer = BlockingScanWriter { entered_tx: None }; + assert!( + tokio::time::timeout(Duration::from_millis(10), shutdown_writer.shutdown()) + .await + .is_err(), + "blocking scan writer shutdown should stay pending" + ); + } + #[tokio::test] async fn test_local_disk_scan_rejects_concurrent_same_prefix_and_releases_on_cancel() { use tempfile::tempdir; @@ -7317,6 +7711,51 @@ mod test { assert_eq!(restored_meta, old_meta); } + #[tokio::test] + async fn test_delete_versions_ignores_missing_non_deleted_version_and_deletes_existing() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let bucket = "bucket"; + let object = "dir/object"; + let existing_version = Uuid::parse_str("10101010-1010-1010-1010-101010101010").expect("version id should parse"); + let missing_version = Uuid::parse_str("20202020-2020-2020-2020-202020202020").expect("version id should parse"); + let existing_data_dir = Uuid::parse_str("30303030-3030-3030-3030-303030303030").expect("data dir should parse"); + let missing_data_dir = Uuid::parse_str("40404040-4040-4040-4040-404040404040").expect("data dir should parse"); + + ensure_test_volume(&disk, bucket).await; + + let object_dir = dir.path().join(bucket).join(object); + fs::create_dir_all(object_dir.join(existing_data_dir.to_string())) + .await + .expect("existing data dir should be created"); + + let existing_fi = test_file_info(object, existing_version, Some(existing_data_dir), None); + let missing_fi = test_file_info(object, missing_version, Some(missing_data_dir), None); + fs::write(object_dir.join(STORAGE_FORMAT_FILE), test_meta(existing_fi.clone())) + .await + .expect("existing metadata should be written"); + + disk.delete_versions_internal(bucket, object, &[missing_fi, existing_fi]) + .await + .expect("missing non-deleted version should not abort deletion"); + + assert!( + matches!( + disk.read_all(bucket, &format!("{object}/{STORAGE_FORMAT_FILE}")).await, + Err(DiskError::FileNotFound) + ), + "metadata should be removed once the remaining real version is deleted" + ); + assert!( + !object_dir.join(existing_data_dir.to_string()).exists(), + "deleted version data directory should leave the object path" + ); + } + #[tokio::test] async fn test_rename_data_failure_before_metadata_commit_preserves_old_metadata() { use tempfile::tempdir; @@ -7411,6 +7850,16 @@ mod test { assert_eq!(err.kind(), ErrorKind::TimedOut); } + #[tokio::test] + async fn local_read_timeout_reader_with_zero_timeout_stays_pending() { + let mut reader = StallTimeoutReader::new(PendingTestReader, Duration::ZERO); + let mut buf = [0; 1]; + + let result = tokio::time::timeout(Duration::from_millis(10), reader.read(&mut buf)).await; + + assert!(result.is_err(), "zero timeout must leave stalled reads pending instead of failing"); + } + #[tokio::test] async fn test_get_disk_id_invalidates_cache_after_format_removal() { use crate::disk::FORMAT_CONFIG_FILE; @@ -7688,6 +8137,53 @@ mod test { assert!(names.contains(&"quux/thud".to_string())); } + #[tokio::test] + async fn test_scan_dir_reports_base_dir_object_metadata() { + use rustfs_filemeta::MetacacheReader; + use tempfile::tempdir; + + let dir = tempdir().expect("operation should succeed"); + let bucket = "test-bucket"; + let base_dir = "base-object"; + let bucket_dir = dir.path().join(bucket); + fs::create_dir_all(bucket_dir.join(base_dir)) + .await + .expect("base object dir should be created"); + fs::write(bucket_dir.join(base_dir).join(STORAGE_FORMAT_FILE), b"meta") + .await + .expect("base object metadata should be written"); + + let endpoint = + Endpoint::try_from(dir.path().to_str().expect("operation should succeed")).expect("operation should succeed"); + let disk = LocalDisk::new(&endpoint, false).await.expect("operation should succeed"); + + let (reader, mut writer) = tokio::io::duplex(4096); + let mut out = MetacacheWriter::new(&mut writer); + let opts = WalkDirOptions { + bucket: bucket.to_string(), + base_dir: base_dir.to_string(), + recursive: false, + ..Default::default() + }; + let mut objs_returned = 0; + + disk.scan_dir(base_dir.to_string(), "".to_string(), &opts, &mut out, &mut objs_returned, false, None) + .await + .expect("operation should succeed"); + out.close().await.expect("operation should succeed"); + + let mut reader = MetacacheReader::new(reader); + let entries = reader.read_all().await.expect("operation should succeed"); + let names: Vec = entries + .into_iter() + .filter(|entry| !entry.metadata.is_empty()) + .map(|entry| entry.name) + .collect(); + + assert_eq!(names, vec![format!("{base_dir}/")]); + assert_eq!(objs_returned, 1); + } + #[tokio::test] async fn test_scan_dir_deduplicates_explicit_dir_marker_recursion() { use rustfs_filemeta::MetacacheReader; @@ -8331,13 +8827,7 @@ mod test { let p = "./testv0"; fs::create_dir_all(&p).await.expect("operation should succeed"); - let ep = match Endpoint::try_from(p) { - Ok(e) => e, - Err(e) => { - println!("{e}"); - return; - } - }; + let ep = Endpoint::try_from(p).expect("endpoint should parse"); let disk = LocalDisk::new(&ep, false).await.expect("operation should succeed"); @@ -8361,13 +8851,7 @@ mod test { let p = "./testv1"; fs::create_dir_all(&p).await.expect("operation should succeed"); - let ep = match Endpoint::try_from(p) { - Ok(e) => e, - Err(e) => { - println!("{e}"); - return; - } - }; + let ep = Endpoint::try_from(p).expect("endpoint should parse"); let disk = LocalDisk::new(&ep, false).await.expect("operation should succeed"); @@ -8696,6 +9180,18 @@ mod test { } } + #[test] + fn test_local_disk_scan_lock_key_combines_base_and_filter_prefixes() { + assert_eq!( + local_disk_scan_lock_key("bucket", "", Some("/prefix/")), + ("bucket".to_string(), "prefix".to_string()) + ); + assert_eq!( + local_disk_scan_lock_key("bucket", "/base/", Some("/prefix/")), + ("bucket".to_string(), "base/prefix".to_string()) + ); + } + #[tokio::test] async fn test_read_file_exists() { let test_file = "./test_read_exists.txt"; @@ -8904,6 +9400,92 @@ mod test { }); } + #[test] + fn direct_io_drive_sync_and_bitrot_retry_envs_respect_overrides() { + temp_env::with_var_unset(ENV_RUSTFS_OBJECT_DIRECT_IO_READ_ENABLE, || { + assert!(!is_direct_io_read_enabled()); + }); + temp_env::with_var(ENV_RUSTFS_OBJECT_DIRECT_IO_READ_ENABLE, Some("true"), || { + assert!(is_direct_io_read_enabled()); + }); + + temp_env::with_var_unset(ENV_RUSTFS_OBJECT_DIRECT_IO_READ_THRESHOLD, || { + assert_eq!(get_direct_io_read_threshold(), DEFAULT_RUSTFS_OBJECT_DIRECT_IO_READ_THRESHOLD); + }); + temp_env::with_var(ENV_RUSTFS_OBJECT_DIRECT_IO_READ_THRESHOLD, Some("12345"), || { + assert_eq!(get_direct_io_read_threshold(), 12_345); + }); + + temp_env::with_var_unset(ENV_RUSTFS_DRIVE_SYNC_ENABLE, || { + assert_eq!( + resolve_durability_mode( + None, + rustfs_utils::get_env_bool(ENV_RUSTFS_DRIVE_SYNC_ENABLE, DEFAULT_RUSTFS_DRIVE_SYNC_ENABLE), + ), + DurabilityMode::Strict + ); + }); + temp_env::with_var(ENV_RUSTFS_DRIVE_SYNC_ENABLE, Some("false"), || { + assert_eq!( + resolve_durability_mode( + None, + rustfs_utils::get_env_bool(ENV_RUSTFS_DRIVE_SYNC_ENABLE, DEFAULT_RUSTFS_DRIVE_SYNC_ENABLE), + ), + DurabilityMode::LegacyOff + ); + }); + + temp_env::with_var_unset(ENV_BITROT_SIZE_MISMATCH_RETRY_COUNT, || { + assert_eq!(bitrot_size_mismatch_retry_count(), DEFAULT_BITROT_SIZE_MISMATCH_RETRY_COUNT as usize); + }); + temp_env::with_var(ENV_BITROT_SIZE_MISMATCH_RETRY_COUNT, Some("7"), || { + assert_eq!(bitrot_size_mismatch_retry_count(), 7); + }); + temp_env::with_var(ENV_BITROT_SIZE_MISMATCH_RETRY_DELAY_MS, Some("42"), || { + assert_eq!(bitrot_size_mismatch_retry_delay(), Duration::from_millis(42)); + }); + } + + #[test] + fn mmap_and_reclaim_metric_helpers_accept_noop_and_positive_paths() { + let metrics = || MmapCopyStageMetrics { + path: "local_test", + access_check_stage: "access", + path_resolve_stage: "path", + metadata_lookup_stage: "metadata_lookup", + metadata_validate_stage: "metadata_validate", + blocking_wait_stage: "blocking_wait", + blocking_task_stage: "blocking_task", + file_open_stage: "file_open", + mmap_map_stage: "mmap_map", + mmap_copy_stage: "mmap_copy", + direct_read_copy_stage: "direct_read_copy", + }; + + record_mmap_copy_stage(metrics(), "mmap_copy", None); + record_mmap_copy_stage(metrics(), "mmap_copy", Some(std::time::Instant::now())); + record_file_cache_reclaim_success("read", 128, std::time::Instant::now()); + record_file_cache_reclaim_error("write"); + + #[cfg(unix)] + { + record_mmap_page_fault_delta("local_test", "mmap_map", MmapPageFaultDelta::default()); + record_mmap_page_fault_delta("local_test", "mmap_map", MmapPageFaultDelta { minor: 1, major: 2 }); + record_direct_read_page_fault_delta("local_test", "direct_read_copy", MmapPageFaultDelta::default()); + record_direct_read_page_fault_delta("local_test", "direct_read_copy", MmapPageFaultDelta { minor: 3, major: 4 }); + } + } + + #[cfg(unix)] + #[test] + fn mmap_page_fault_counts_respect_disabled_and_enabled_modes() { + assert_eq!(read_mmap_page_fault_counts(false), None); + + let counts = read_mmap_page_fault_counts(true).expect("getrusage should return page fault counters"); + assert!(counts.minor >= 0); + assert!(counts.major >= 0); + } + #[cfg(unix)] #[test] fn mmap_page_size_is_cached_positive() { @@ -8941,6 +9523,156 @@ mod test { .await; } + #[test] + fn resolve_local_disk_root_reports_missing_path_as_volume_not_found() { + let dir = tempfile::tempdir().expect("temp dir should be created"); + let missing = dir.path().join("missing"); + + let err = resolve_local_disk_root(missing.to_str().expect("temp path should be utf8")) + .expect_err("missing disk root must be rejected"); + + assert!(matches!(err, DiskError::VolumeNotFound)); + } + + #[tokio::test] + async fn local_disk_debug_includes_stable_identity_fields() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let endpoint = Endpoint::try_from(dir.path().to_string_lossy().as_ref()).expect("endpoint should parse"); + let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created"); + + let debug = format!("{disk:?}"); + + assert!(debug.contains("LocalDisk")); + assert!(debug.contains("root")); + assert!(debug.contains("format_path")); + assert!(debug.contains("endpoint")); + } + + #[tokio::test] + async fn std_backend_truncate_append_stream_and_full_read_restore_bytes() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let volume = "test-volume"; + fs::create_dir_all(dir.path().join(volume)) + .await + .expect("volume should be created"); + let backend = StdBackend::new(dir.path().to_path_buf()); + + let mut writer = backend + .open_write(volume, "nested/blob.bin", WriteMode::Truncate { size_hint: 6 }) + .await + .expect("truncate writer should open"); + writer.write_all(b"abcdef").await.expect("initial bytes should write"); + writer.shutdown().await.expect("truncate writer should shutdown"); + + let window = backend + .pread_bytes(volume, "nested/blob.bin", 1, 3, None) + .await + .expect("pread should restore requested window"); + assert_eq!(window, Bytes::from_static(b"bcd")); + + let mut writer = backend + .open_write(volume, "nested/blob.bin", WriteMode::Truncate { size_hint: 2 }) + .await + .expect("second truncate writer should open"); + writer.write_all(b"xy").await.expect("truncated bytes should write"); + writer.shutdown().await.expect("second truncate writer should shutdown"); + + let mut writer = backend + .open_write(volume, "nested/blob.bin", WriteMode::Append) + .await + .expect("append writer should open"); + writer.write_all(b"z").await.expect("append byte should write"); + writer.shutdown().await.expect("append writer should shutdown"); + + let mut stream = backend + .open_read_stream(volume, "nested/blob.bin", 0, 3) + .await + .expect("bounded stream should open"); + let mut streamed = Vec::new(); + stream.read_to_end(&mut streamed).await.expect("bounded stream should read"); + assert_eq!(streamed, b"xyz"); + + let mut full = backend + .open_full_read(volume, "nested/blob.bin") + .await + .expect("full stream should open"); + let mut body = Vec::new(); + full.read_to_end(&mut body).await.expect("full stream should read"); + assert_eq!(body, b"xyz"); + } + + #[tokio::test] + async fn std_backend_rejects_overflow_and_out_of_bounds_reads() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let volume = "test-volume"; + fs::create_dir_all(dir.path().join(volume)) + .await + .expect("volume should be created"); + fs::write(dir.path().join(volume).join("blob.bin"), b"abc") + .await + .expect("test object should be written"); + let backend = StdBackend::new(dir.path().to_path_buf()); + + let overflow = backend.pread_bytes(volume, "blob.bin", usize::MAX, 1, None).await; + assert!(matches!(overflow, Err(DiskError::FileCorrupt))); + + let out_of_bounds = backend.pread_bytes(volume, "blob.bin", 2, 2, None).await; + assert!(matches!(out_of_bounds, Err(DiskError::FileCorrupt))); + + let stream_out_of_bounds = backend.open_read_stream(volume, "blob.bin", 2, 2).await; + assert!(matches!(stream_out_of_bounds, Err(DiskError::FileCorrupt))); + } + + #[tokio::test] + async fn file_cache_reclaim_wrappers_forward_read_write_flush_and_shutdown() { + use futures_util::task::noop_waker_ref; + use std::io::IoSlice; + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let path = dir.path().join("reclaim.bin"); + let file = File::create(&path).await.expect("writer file should be created"); + let mut writer = FileCacheReclaimWriter::new(file, 6, true); + + let mut cx = Context::from_waker(noop_waker_ref()); + let bufs = [IoSlice::new(b"ab"), IoSlice::new(b"cd")]; + let vectored = Pin::new(&mut writer).poll_write_vectored(&mut cx, &bufs); + assert!(matches!(vectored, Poll::Ready(Ok(_)) | Poll::Pending)); + let _ = AsyncWrite::is_write_vectored(&writer); + + writer.write_all(b"abcdef").await.expect("writer should forward writes"); + writer.flush().await.expect("writer should forward flush"); + writer.shutdown().await.expect("writer should reclaim on shutdown"); + + let file = File::open(&path).await.expect("reader file should open"); + let mut reader = FileCacheReclaimReader::new(file, 0, 6, true); + let mut body = Vec::new(); + reader.read_to_end(&mut body).await.expect("reader should forward reads"); + assert!(body.ends_with(b"abcdef")); + } + + #[cfg(target_os = "macos")] + #[tokio::test] + async fn macos_nocache_helpers_accept_tokio_and_std_files() { + use tempfile::tempdir; + + let dir = tempdir().expect("temp dir should be created"); + let path = dir.path().join("nocache.bin"); + fs::write(&path, b"nocache").await.expect("test file should be written"); + + let tokio_file = File::open(&path).await.expect("tokio file should open"); + set_fd_nocache(&tokio_file).expect("tokio fd should accept F_NOCACHE"); + + let std_file = std::fs::File::open(&path).expect("std file should open"); + set_std_fd_nocache(&std_file).expect("std fd should accept F_NOCACHE"); + } + #[cfg(unix)] #[test] fn mmap_page_fault_delta_clamps_non_monotonic_counts() { diff --git a/crates/ecstore/src/ecstore_validation_blackbox.rs b/crates/ecstore/src/ecstore_validation_blackbox.rs new file mode 100644 index 000000000..094445d9f --- /dev/null +++ b/crates/ecstore/src/ecstore_validation_blackbox.rs @@ -0,0 +1,434 @@ +use crate::disk::endpoint::Endpoint; +use crate::disk::format::FormatV3; +use crate::disk::{DiskAPI, DiskOption, DiskStore, WalkDirOptions, new_disk}; +use crate::error::Error; +use crate::io_support::rio::HashReader; +use crate::object_api::{BLOCK_SIZE_V2, ObjectOptions, PutObjReader}; +use crate::set_disk::SetDisks; +use crate::storage_api_contracts::bucket::{BucketOperations as _, MakeBucketOptions}; +use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations as _}; +use crate::storage_api_contracts::range::HTTPRangeSpec; +use crate::store::init_format::save_format_file; +use http::HeaderMap; +use rustfs_filemeta::{MetacacheReader, MetacacheWriter}; +use std::io::Cursor; +use std::mem; +use std::path::PathBuf; +use std::sync::Arc; +use tokio::fs; +use tokio::io::AsyncReadExt; +use tokio::sync::RwLock; + +async fn make_local_set_disks(drive_count: usize, parity_count: usize) -> Arc { + let format = FormatV3::new(1, drive_count); + let mut endpoints = Vec::with_capacity(drive_count); + let mut disks = Vec::with_capacity(drive_count); + + for disk_idx in 0..drive_count { + let dir = tempfile::tempdir().expect("tempdir should be created"); + let mut endpoint = + Endpoint::try_from(dir.path().to_str().expect("tempdir path should be utf8")).expect("endpoint should parse"); + endpoint.set_pool_index(0); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_idx); + + let disk = new_disk( + &endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("disk should be created"); + + let mut disk_format = format.clone(); + disk_format.erasure.this = format.erasure.sets[0][disk_idx]; + save_format_file(&Some(disk.clone()), &Some(disk_format)) + .await + .expect("format should be saved"); + + mem::forget(dir); + endpoints.push(endpoint); + disks.push(Some(disk)); + } + + SetDisks::new( + "ecstore-validation-blackbox".to_string(), + Arc::new(RwLock::new(disks)), + drive_count, + parity_count, + 0, + 0, + endpoints, + format, + Vec::new(), + ) + .await +} + +async fn first_shard_part_path(disk: &DiskStore, bucket: &str, object: &str) -> PathBuf { + let object_dir = disk.path().join(bucket).join(object); + let mut entries = fs::read_dir(&object_dir) + .await + .unwrap_or_else(|err| panic!("object data dir should be readable at {object_dir:?}: {err}")); + while let Some(entry) = entries.next_entry().await.expect("object data dir entry should be readable") { + if entry + .file_type() + .await + .expect("object data entry type should be readable") + .is_dir() + { + let part_path = entry.path().join("part.1"); + if fs::metadata(&part_path).await.is_ok() { + return part_path; + } + } + } + panic!("part.1 shard should exist under {object_dir:?}"); +} + +async fn shard_part_paths(set_disks: &Arc, bucket: &str, object: &str) -> Vec { + let disks = set_disks.disks.read().await; + let mut paths = Vec::new(); + for disk in disks.iter().flatten() { + paths.push(first_shard_part_path(disk, bucket, object).await); + } + paths +} + +#[tokio::test] +async fn blackbox_put_unknown_actual_size_restores_body_and_records_written_size() { + let set_disks = make_local_set_disks(4, 2).await; + let bucket = "bb-unknown-actual-size"; + let object = "object.bin"; + let payload = (0..(BLOCK_SIZE_V2 + 123)) + .map(|idx| ((idx * 17) % 251) as u8) + .collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let stream = HashReader::from_stream(Cursor::new(payload.clone()), payload.len() as i64, -1, None, None, false) + .expect("hash reader should accept unknown actual size"); + let mut reader = PutObjReader::new(stream); + let written = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written"); + + assert_eq!(written.size, payload.len() as i64); + assert_eq!(written.parts[0].actual_size, payload.len() as i64); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("object reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("object should stream"); + assert_eq!(restored, payload); +} + +#[tokio::test] +async fn blackbox_get_restores_body_after_one_shard_file_is_removed() { + let set_disks = make_local_set_disks(4, 2).await; + let bucket = "bb-missing-shard-file"; + let object = "object.bin"; + let payload = (0..(BLOCK_SIZE_V2 + 321)) + .map(|idx| ((idx * 19) % 251) as u8) + .collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written"); + + let disk = { + let disks = set_disks.disks.read().await; + disks[0].clone().expect("first disk should exist") + }; + let shard_path = first_shard_part_path(&disk, bucket, object).await; + fs::remove_file(&shard_path) + .await + .unwrap_or_else(|err| panic!("test shard should be removable at {shard_path:?}: {err}")); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("object should remain readable after one shard file is removed"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("degraded object should stream"); + + assert_eq!(restored, payload); +} + +#[tokio::test] +async fn blackbox_get_restores_body_and_enqueues_repair_after_one_corrupt_shard() { + let set_disks = make_local_set_disks(4, 2).await; + let bucket = "bb-corrupt-shard-repair"; + let object = "object.bin"; + let payload = (0..(BLOCK_SIZE_V2 + 777)) + .map(|idx| ((idx * 29) % 251) as u8) + .collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written"); + + let paths = shard_part_paths(&set_disks, bucket, object).await; + fs::write(&paths[0], b"corrupt shard bytes") + .await + .unwrap_or_else(|err| panic!("test shard should be corruptible at {:?}: {err}", paths[0])); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("object should remain readable after one corrupt shard"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("corrupt-shard object should stream from parity"); + + assert_eq!(restored, payload); +} + +#[tokio::test] +async fn blackbox_range_read_restores_exact_slice_with_one_offline_disk() { + let set_disks = make_local_set_disks(4, 2).await; + let bucket = "bb-range-offline-disk"; + let object = "object.bin"; + let payload = (0..(BLOCK_SIZE_V2 + 4096)) + .map(|idx| ((idx * 23) % 251) as u8) + .collect::>(); + let range_start = 513usize; + let range_len = 8192usize; + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written"); + + { + let mut disks = set_disks.disks.write().await; + disks[2] = None; + } + + let range = HTTPRangeSpec { + is_suffix_length: false, + start: range_start as i64, + end: (range_start + range_len - 1) as i64, + }; + let mut get_reader = set_disks + .get_object_reader(bucket, object, Some(range), HeaderMap::new(), &opts) + .await + .expect("range reader should open with one offline disk"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("range body should stream"); + + assert_eq!(restored, payload[range_start..range_start + range_len]); +} + +#[tokio::test] +async fn blackbox_delete_marker_hides_object_body_without_erasing_prior_version_metadata() { + let set_disks = make_local_set_disks(4, 2).await; + let bucket = "bb-delete-marker-read-negative"; + let object = "object.bin"; + let opts = ObjectOptions { + no_lock: true, + version_suspended: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"body hidden by delete marker".to_vec()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written before delete marker"); + + let marker = set_disks + .delete_object(bucket, object, opts.clone()) + .await + .expect("version suspended delete should create a marker"); + assert!(marker.delete_marker); + + let err = match set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("delete marker must hide the object body"), + Err(err) => err, + }; + assert!( + matches!(err, Error::ObjectNotFound(_, _) | Error::MethodNotAllowed), + "delete marker read must fail closed, got {err:?}" + ); +} + +#[tokio::test] +async fn blackbox_local_disk_walk_dir_emits_metadata_entries_with_prefix_forward_and_limit() { + let dir = tempfile::tempdir().expect("tempdir should be created"); + let bucket = "bb-local-walk"; + let endpoint = Endpoint::try_from(dir.path().to_str().expect("tempdir path should be utf8")).expect("endpoint should parse"); + let disk = new_disk( + &endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("disk should be created"); + disk.make_volume(bucket).await.expect("bucket volume should be created"); + + for object in ["prefix/a", "prefix/b", "prefix/c", "other/d"] { + disk.write_all(bucket, &format!("{object}/xl.meta"), bytes::Bytes::from_static(b"meta")) + .await + .expect("metadata object should be written"); + } + + let (reader, mut writer) = tokio::io::duplex(4096); + let opts = WalkDirOptions { + bucket: bucket.to_string(), + base_dir: "prefix/".to_string(), + recursive: true, + forward_to: Some("prefix/b".to_string()), + limit: 2, + ..Default::default() + }; + + disk.walk_dir(opts, &mut writer) + .await + .expect("walk_dir should stream metadata entries"); + MetacacheWriter::new(&mut writer) + .close() + .await + .expect("metacache stream should close"); + drop(writer); + + let mut reader = MetacacheReader::new(reader); + let entries = reader.read_all().await.expect("metacache stream should decode"); + let names = entries + .into_iter() + .filter(|entry| !entry.metadata.is_empty()) + .map(|entry| entry.name) + .collect::>(); + + assert_eq!(names, vec!["prefix/b".to_string(), "prefix/c".to_string()]); +} + +#[tokio::test] +#[serial_test::serial] +async fn blackbox_issue3031_diag_covers_put_success_cleanup_and_error_summary() { + temp_env::async_with_vars([("RUSTFS_ISSUE3031_DIAG_ENABLE", Some("true"))], async { + let set_disks = make_local_set_disks(4, 2).await; + let bucket = "bb-issue3031-diag"; + let object = "object.bin"; + let first_payload = b"first diagnostic body".to_vec(); + let second_payload = b"second diagnostic body that replaces the first".to_vec(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + + let mut first_reader = PutObjReader::from_vec(first_payload); + set_disks + .put_object(bucket, object, &mut first_reader, &opts) + .await + .expect("first diagnostic write should succeed"); + + let mut second_reader = PutObjReader::from_vec(second_payload.clone()); + set_disks + .put_object(bucket, object, &mut second_reader, &opts) + .await + .expect("overwrite should succeed and report cleanup-present diagnostics"); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("overwritten object reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("overwritten object should stream"); + assert_eq!(restored, second_payload); + + let failed_set = make_local_set_disks(1, 0).await; + let failed_bucket = "bb-issue3031-fail"; + failed_set + .make_bucket(failed_bucket, &MakeBucketOptions::default()) + .await + .expect("failure bucket should be created"); + { + let mut disks = failed_set.disks.write().await; + disks[0] = None; + } + let mut failed_reader = PutObjReader::from_vec(b"must fail closed".to_vec()); + let err = failed_set + .put_object(failed_bucket, "object.bin", &mut failed_reader, &opts) + .await + .expect_err("offline only disk must fail writer setup under diagnostics"); + assert!( + matches!(err, Error::InsufficientWriteQuorum(_, _) | Error::ErasureWriteQuorum), + "diagnostic failure should still report a write-quorum style error, got {err:?}" + ); + }) + .await; +} diff --git a/crates/ecstore/src/erasure/codec/bridge.rs b/crates/ecstore/src/erasure/codec/bridge.rs index 94aa023e1..05433cd79 100644 --- a/crates/ecstore/src/erasure/codec/bridge.rs +++ b/crates/ecstore/src/erasure/codec/bridge.rs @@ -659,6 +659,76 @@ mod tests { assert!(err.to_string().contains("inconsistent read source shards")); } + #[test] + fn rustfs_codec_decode_engine_rebuilds_missing_parity_for_source_verification() { + let erasure = Erasure::new(2, 3, 32); + let encoded = erasure + .encode_data(&(0u8..64u8).collect::>()) + .expect("test stripe should encode"); + let mut shards = encoded.into_iter().map(|shard| Some(shard.to_vec())).collect::>(); + shards[0] = None; + let missing_parity_index = erasure.data_shards + erasure.parity_shards - 1; + shards[missing_parity_index] = None; + + let engine = RustfsCodecDecodeEngine::new(&erasure).expect("engine should be created"); + let mut workspace = engine + .prepare_workspace(erasure.shard_size()) + .expect("workspace should be prepared"); + let outcome = engine + .reconstruct_into(&mut shards, &mut workspace) + .expect("missing data plus missing parity should be recoverable"); + + assert_eq!(outcome, GET_RECONSTRUCT_OUTCOME_RUSTFS_CALLED); + assert!(shards[0].is_some()); + assert!(shards[missing_parity_index].is_some()); + } + + #[test] + fn rustfs_codec_decode_engine_rejects_invalid_empty_payload_shape() { + let erasure = Erasure::new(2, 2, 16); + let engine = RustfsCodecDecodeEngine::new(&erasure).expect("engine should be created"); + let mut workspace = engine.prepare_workspace(0).expect("workspace should be prepared"); + let mut shards = vec![Some(Vec::new()), None, Some(Vec::new())]; + + let err = engine + .reconstruct_into(&mut shards, &mut workspace) + .expect_err("invalid empty payload shard count must fail closed"); + + assert!(err.to_string().contains("invalid shard count")); + } + + #[test] + fn rustfs_codec_decode_engine_skips_empty_payload_when_enough_empty_sources_exist() { + let erasure = Erasure::new(3, 2, 16); + let engine = RustfsCodecDecodeEngine::new(&erasure).expect("engine should be created"); + let mut workspace = engine.prepare_workspace(0).expect("workspace should be prepared"); + let mut shards = vec![Some(Vec::new()), None, Some(Vec::new()), Some(Vec::new()), None]; + + let outcome = engine + .reconstruct_into(&mut shards, &mut workspace) + .expect("empty payload should be restored without codec allocation"); + + assert_eq!(outcome, GET_RECONSTRUCT_OUTCOME_SKIP_EMPTY_PAYLOAD); + assert_eq!(shards[1], Some(Vec::new())); + assert!(!engine.codec_is_initialized()); + } + + #[test] + fn rustfs_codec_decode_engine_returns_called_without_parity_codec() { + let erasure = Erasure::new(2, 0, 16); + let engine = RustfsCodecDecodeEngine::new(&erasure).expect("engine should be created"); + let mut workspace = engine.prepare_workspace(4).expect("workspace should be prepared"); + let mut shards = vec![None, Some(vec![2, 2, 2, 2])]; + + let outcome = engine + .reconstruct_into(&mut shards, &mut workspace) + .expect("zero parity engine should not allocate codec"); + + assert_eq!(outcome, GET_RECONSTRUCT_OUTCOME_RUSTFS_CALLED); + assert!(shards[0].is_none()); + assert!(!engine.codec_is_initialized()); + } + #[test] fn rustfs_codec_decode_engine_recovers_empty_data_shard() { let erasure = Erasure::new(4, 2, 16); @@ -672,6 +742,34 @@ mod tests { assert!(shards.iter().take(erasure.data_shards).all(Option::is_some)); } + #[test] + fn codec_streaming_decode_engine_dispatches_to_legacy_workspace_and_rejects_mismatch() { + let erasure = Erasure::new(2, 2, 16); + let mut shards = encoded_shards(&erasure, b"legacy enum dispatch"); + shards[0] = None; + + let engine = CodecStreamingDecodeEngine::legacy(erasure); + assert_eq!(engine.data_shards(), 2); + assert_eq!(engine.parity_shards(), 2); + assert_eq!(engine.block_size(), 16); + assert_eq!(engine.engine_name(), GET_CODEC_STREAMING_ENGINE_LEGACY); + assert!(!engine.supports_progressive_decode()); + assert!(!engine.supports_aligned_shards()); + + let mut workspace = engine.prepare_workspace(8).expect("workspace should be prepared"); + assert_eq!(workspace.shard_len(), 8); + engine + .reconstruct_into(&mut shards, &mut workspace) + .expect("legacy enum engine should dispatch reconstruction"); + assert!(shards.iter().take(engine.data_shards()).all(Option::is_some)); + + let rustfs = CodecStreamingDecodeEngine::rustfs(&Erasure::new(2, 2, 16)).expect("engine should be created"); + let err = rustfs + .reconstruct_into(&mut shards, &mut workspace) + .expect_err("engine/workspace variant mismatch must fail"); + assert!(err.to_string().contains("engine/workspace mismatch")); + } + #[test] fn codec_streaming_decode_engine_dispatches_to_rustfs_workspace() { let erasure = Erasure::new(4, 2, 16); @@ -680,6 +778,9 @@ mod tests { let engine = CodecStreamingDecodeEngine::rustfs(&erasure).expect("engine should be created"); let mut workspace = engine.prepare_workspace(4).expect("workspace should be prepared"); + if let CodecStreamingDecodeWorkspace::Rustfs(workspace) = &workspace { + assert_eq!(::shard_len(workspace), 4); + } engine .reconstruct_into(&mut shards, &mut workspace) .expect("enum engine should dispatch reconstruction"); diff --git a/crates/ecstore/src/erasure/codec/workspace.rs b/crates/ecstore/src/erasure/codec/workspace.rs index 7d05700b1..f0d6af58a 100644 --- a/crates/ecstore/src/erasure/codec/workspace.rs +++ b/crates/ecstore/src/erasure/codec/workspace.rs @@ -102,4 +102,17 @@ mod tests { assert_eq!(buf.len(), 4); assert_eq!(pool.buffers.len(), 4); } + + #[test] + fn workspace_reports_shard_len_and_pool_reserves_when_reused_slot_is_too_small() { + let workspace = RustfsCodecDecodeWorkspace::new(37); + assert_eq!(workspace.shard_len(), 37); + + let mut pool = ShardBufferPool::new(1); + pool.put(0, Vec::with_capacity(2)); + let grown = pool.take(0, 8); + + assert_eq!(grown.len(), 8); + assert!(grown.capacity() >= 8); + } } diff --git a/crates/ecstore/src/erasure/coding/bitrot.rs b/crates/ecstore/src/erasure/coding/bitrot.rs index 45349b59c..038ed6d84 100644 --- a/crates/ecstore/src/erasure/coding/bitrot.rs +++ b/crates/ecstore/src/erasure/coding/bitrot.rs @@ -533,10 +533,9 @@ impl BitrotWriterWrapper { #[cfg(test)] mod tests { - - use super::BitrotReader; - use super::BitrotWriter; - use super::bitrot_shard_file_size; + use super::{ + BitrotReader, BitrotWriter, BitrotWriterWrapper, CustomWriter, bitrot_shard_file_size, bitrot_verify, write_all_vectored, + }; use rustfs_utils::HashAlgorithm; use std::io::{Cursor, IoSlice}; use std::sync::{ @@ -544,7 +543,7 @@ mod tests { atomic::{AtomicUsize, Ordering}, }; use std::task::{Context, Poll}; - use tokio::io::AsyncWrite; + use tokio::io::{AsyncWrite, AsyncWriteExt}; #[derive(Default)] struct VectoredCountingWriter { @@ -607,6 +606,78 @@ mod tests { } } + #[derive(Default)] + struct LimitedVectoredWriter { + max_write: usize, + writes: Vec, + } + + impl AsyncWrite for LimitedVectoredWriter { + fn poll_write(mut self: std::pin::Pin<&mut Self>, _cx: &mut Context<'_>, buf: &[u8]) -> Poll> { + let len = buf.len().min(self.max_write); + self.writes.extend_from_slice(&buf[..len]); + Poll::Ready(Ok(len)) + } + + fn poll_flush(self: std::pin::Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_shutdown(self: std::pin::Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_write_vectored( + mut self: std::pin::Pin<&mut Self>, + _cx: &mut Context<'_>, + bufs: &[IoSlice<'_>], + ) -> Poll> { + let mut remaining = self.max_write; + let mut written = 0; + for buf in bufs { + if remaining == 0 { + break; + } + let len = buf.len().min(remaining); + self.writes.extend_from_slice(&buf[..len]); + remaining -= len; + written += len; + } + Poll::Ready(Ok(written)) + } + + fn is_write_vectored(&self) -> bool { + true + } + } + + #[tokio::test] + async fn vectored_test_writers_cover_fallback_flush_and_shutdown_paths() { + let mut counting = VectoredCountingWriter::default(); + assert!(counting.is_write_vectored()); + let err = counting + .write(b"plain write") + .await + .expect_err("plain writes should be rejected by vectored-only test writer"); + assert_eq!(err.to_string(), "poll_write should not be used"); + counting.flush().await.expect("flush should succeed"); + counting.shutdown().await.expect("shutdown should succeed"); + + let mut limited = LimitedVectoredWriter { + max_write: 2, + writes: Vec::new(), + }; + assert!(limited.is_write_vectored()); + let written = limited + .write(b"plain") + .await + .expect("limited writer should accept partial plain write"); + assert_eq!(written, 2); + assert_eq!(limited.writes, b"pl"); + limited.flush().await.expect("flush should succeed"); + limited.shutdown().await.expect("shutdown should succeed"); + } + #[tokio::test] async fn test_bitrot_read_write_ok() { let data = b"hello world! this is a test shard."; @@ -647,6 +718,94 @@ mod tests { assert_eq!(data, &out[..]); } + #[tokio::test] + async fn bitrot_verify_accepts_valid_shard_file_and_rejects_size_or_hash_mismatch() { + let data = b"bitrot verify covers every shard"; + let shard_size = 8; + let algo = HashAlgorithm::HighwayHash256S; + let writer = Cursor::new(Vec::new()); + let mut bitrot_writer = BitrotWriter::new(writer, shard_size, algo.clone()); + for chunk in data.chunks(shard_size) { + bitrot_writer.write(chunk).await.unwrap(); + } + let written = bitrot_writer.into_inner().into_inner(); + + bitrot_verify(Cursor::new(written.clone()), written.len(), data.len(), algo.clone(), shard_size) + .await + .expect("valid bitrot shard file should verify"); + + let err = bitrot_verify(Cursor::new(written.clone()), written.len() - 1, data.len(), algo.clone(), shard_size) + .await + .expect_err("wrong file size must be rejected before reading data"); + assert!(err.to_string().contains("size mismatch")); + + let mut corrupt = written; + let last = corrupt.len() - 1; + corrupt[last] ^= 0x80; + let err = bitrot_verify( + Cursor::new(corrupt), + super::bitrot_shard_file_size(data.len(), shard_size, algo.clone()), + data.len(), + algo, + shard_size, + ) + .await + .expect_err("hash mismatch must reject corrupted data"); + assert!(err.to_string().contains("hash mismatch")); + } + + #[tokio::test] + async fn write_all_vectored_retries_partial_hash_and_data_writes_and_rejects_zero_write() { + let mut writer = LimitedVectoredWriter { + max_write: 2, + writes: Vec::new(), + }; + + write_all_vectored(&mut writer, b"hash", b"payload").await.unwrap(); + assert_eq!(writer.writes, b"hashpayload"); + + let mut zero_writer = LimitedVectoredWriter { + max_write: 0, + writes: Vec::new(), + }; + let err = write_all_vectored(&mut zero_writer, b"hash", b"payload") + .await + .expect_err("zero-byte vectored writes must fail"); + assert_eq!(err.kind(), std::io::ErrorKind::WriteZero); + } + + #[tokio::test] + async fn bitrot_reader_rejects_output_buffers_larger_than_shard_size() { + let mut reader = BitrotReader::new(Cursor::new(Vec::::new()), 4, HashAlgorithm::None, false); + let mut out = [0u8; 5]; + let err = reader + .read(&mut out) + .await + .expect_err("oversized output buffers must be rejected before reading"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!(err.to_string().contains("exceeds shard size")); + } + + #[tokio::test] + async fn custom_writer_other_forwards_io_and_wrapper_reports_non_inline_state() { + let writer = CountingWriter::default(); + let mut custom = CustomWriter::new_tokio_writer(writer); + assert!(custom.get_inline_data().is_none()); + assert!(!custom.is_write_vectored()); + custom.write_all(b"abc").await.unwrap(); + custom.flush().await.unwrap(); + custom.shutdown().await.unwrap(); + assert!(custom.into_inline_data().is_none()); + + let other = BitrotWriterWrapper::new(CustomWriter::new_tokio_writer(CountingWriter::default()), 8, HashAlgorithm::None); + assert!(format!("{other:?}").contains("Other")); + assert!(other.into_inline_data().is_none()); + + let inline = BitrotWriterWrapper::new(CustomWriter::new_inline_buffer(), 8, HashAlgorithm::None); + assert!(format!("{inline:?}").contains("InlineBuffer")); + } + #[tokio::test] async fn test_bitrot_read_hash_mismatch() { let data = b"test data for bitrot"; diff --git a/crates/ecstore/src/erasure/coding/decode.rs b/crates/ecstore/src/erasure/coding/decode.rs index ad93d9e64..2b97c8494 100644 --- a/crates/ecstore/src/erasure/coding/decode.rs +++ b/crates/ecstore/src/erasure/coding/decode.rs @@ -755,12 +755,6 @@ where self.buffers.ensure_slots(num_readers); let mut retire_readers = Vec::new(); - let mut unavailable_data_sources = self - .readers - .iter() - .take(self.data_shards) - .map(|reader| reader.is_none()) - .collect::>(); if num_readers >= self.data_shards { let mut reader_iter = ReaderLaunchIter::new(&mut self.readers, read_costs, locality_preference_enabled); let mut sets = FuturesUnordered::new(); @@ -808,8 +802,6 @@ where let mut first_shard_recorded = false; let mut pending = sets.len(); let mut scheduled_all = false; - let verification_success_target = self.total_shards.min(self.data_shards + 1); - let parity_shards = self.total_shards.saturating_sub(self.data_shards); loop { let item = if !scheduled_all { match shard_read_hedge_delay(self.read_timeout) { @@ -888,9 +880,6 @@ where result, should_retire, ); - if self.verify_reconstruction && i < self.data_shards && result_is_err { - unavailable_data_sources[i] = true; - } if result_is_err { failed += 1; if let Some((next_i, next_reader)) = reader_iter.next() { @@ -929,73 +918,11 @@ where } } - let mut missing_data_sources = unavailable_data_sources.iter().filter(|missing| **missing).count(); - if self.verify_reconstruction && success >= self.data_shards { - for (idx, active) in active_readers.iter().take(self.data_shards).enumerate() { - if *active && !unavailable_data_sources[idx] { - missing_data_sources += 1; - } - } - } - - let needs_reconstruction_verification = self.verify_reconstruction - && verification_success_target > self.data_shards - && missing_data_sources > 0 - && missing_data_sources < parity_shards; - - let target_success = if needs_reconstruction_verification { - verification_success_target - } else { - self.data_shards - }; - - while success + pending < target_success { - if let Some((next_i, next_reader)) = reader_iter.next() { - let has_reader = next_reader.is_some(); - let recycled_buf = if has_reader { - Some(self.buffers.take(next_i, shard_size)) - } else { - None - }; - let next_read_cost = read_costs.get(next_i).copied().unwrap_or(ShardReadCost::Unknown); - record_scheduled_read_cost( - next_read_cost, - locality_preference_enabled, - low_cost_available, - self.data_shards, - true, - &mut local_preferred, - &mut remote_scheduled, - &mut fallback_to_remote, - ); - scheduled += 1; - active_readers[next_i] = has_reader; - pending += 1; - sets.push(read_shard( - next_i, - next_read_cost, - next_reader, - recycled_buf, - shard_size, - self.data_shards, - self.read_timeout, - self.metrics_path, - )); - } else { - scheduled_all = true; - break; - } - } - - if success >= target_success { + if success >= self.data_shards { break; } - if success >= self.data_shards && (!needs_reconstruction_verification || success + pending < target_success) { - break; - } - - if success + pending < target_success { + if success + pending < self.data_shards { break; } } @@ -1869,6 +1796,7 @@ mod tests { erasure::coding::{BitrotReader, BitrotWriter}, }; use rustfs_utils::HashAlgorithm; + use std::future::Future; use std::io::Cursor; use std::pin::Pin; use std::sync::{ @@ -1877,6 +1805,7 @@ mod tests { }; use std::task::{Context, Poll}; use tokio::io::ReadBuf; + use tokio::time::{Instant as TokioInstant, Sleep}; type BoxedShardReader = Box; @@ -1949,8 +1878,16 @@ mod tests { enum TestShardReader { Ready(Cursor>), + ReadyAt { + cursor: Cursor>, + ready_at: TokioInstant, + sleep: Option>>, + }, Pending, - PartialThenPending { data: Vec, emitted: bool }, + PartialThenPending { + data: Vec, + emitted: bool, + }, TimedOut, } @@ -1958,6 +1895,15 @@ mod tests { fn poll_read(mut self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &mut ReadBuf<'_>) -> Poll> { match &mut *self { TestShardReader::Ready(cursor) => Pin::new(cursor).poll_read(cx, buf), + TestShardReader::ReadyAt { cursor, ready_at, sleep } => { + if TokioInstant::now() < *ready_at { + let sleeper = sleep.get_or_insert_with(|| Box::pin(tokio::time::sleep_until(*ready_at))); + if sleeper.as_mut().poll(cx).is_pending() { + return Poll::Pending; + } + } + Pin::new(cursor).poll_read(cx, buf) + } TestShardReader::Pending => { cx.waker().wake_by_ref(); Poll::Pending @@ -1978,6 +1924,49 @@ mod tests { } } + struct FailingEmitWriter; + + impl AsyncWrite for FailingEmitWriter { + fn poll_write(self: Pin<&mut Self>, _cx: &mut Context<'_>, _buf: &[u8]) -> Poll> { + Poll::Ready(Err(io::Error::new(ErrorKind::BrokenPipe, "injected emit failure"))) + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + } + + #[test] + fn parallel_reader_constructor_variants_preserve_read_cost_and_verification_flags() { + let erasure = Erasure::new(2, 1, 64); + let readers = vec![None, None, None]; + let read_costs = vec![ShardReadCost::Local, ShardReadCost::Remote, ShardReadCost::Unknown]; + + let reader: ParallelReader>> = + ParallelReader::new_with_metrics_path_read_costs_and_reconstruction_verification( + readers, + erasure.clone(), + 64, + 17, + Some("constructor-test"), + read_costs.clone(), + ); + + assert_eq!(reader.offset, reader.shard_size); + assert_eq!(reader.read_costs, read_costs); + assert!(reader.verify_reconstruction); + + let defaulted: ParallelReader>> = + ParallelReader::new_with_metrics_path_and_reconstruction_verification(vec![None, None, None], erasure, 0, 64, None); + + assert_eq!(defaulted.read_costs, vec![ShardReadCost::Unknown; 3]); + assert!(defaulted.verify_reconstruction); + } + #[tokio::test] async fn test_write_data_blocks_writes_range_across_blocks() { let blocks = vec![Some(vec![1, 2, 3, 4]), Some(vec![5, 6, 7]), Some(vec![8, 9])]; @@ -2011,6 +2000,111 @@ mod tests { assert!(out.is_empty()); } + #[tokio::test] + async fn test_write_data_blocks_rejects_offset_length_overflow() { + let blocks = vec![Some(vec![1, 2, 3, 4])]; + let mut out = Vec::new(); + + let err = write_data_blocks(&mut out, &blocks, 1, usize::MAX, 1).await.unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!(out.is_empty()); + } + + #[tokio::test] + async fn test_write_data_blocks_rejects_missing_data_shard_even_when_total_bytes_are_available() { + let blocks = vec![None, Some(vec![1, 2, 3, 4])]; + let mut out = Vec::new(); + + let err = write_data_blocks(&mut out, &blocks, 2, 0, 1).await.unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::UnexpectedEof); + assert!(out.is_empty()); + } + + #[tokio::test] + async fn test_write_data_blocks_propagates_writer_emit_failure() { + let blocks = vec![Some(vec![1, 2, 3, 4])]; + let mut writer = FailingEmitWriter; + + let err = write_data_blocks(&mut writer, &blocks, 1, 0, 4) + .await + .expect_err("writer failure must fail the decoded emit path"); + + assert_eq!(err.kind(), ErrorKind::BrokenPipe); + assert_eq!(err.to_string(), "injected emit failure"); + } + + #[tokio::test] + async fn test_erasure_decode_rejects_reader_count_and_range_overflow() { + let erasure = Erasure::new(2, 1, 64); + let mut output = Vec::new(); + + let (written, err) = erasure + .decode(&mut output, Vec::>>>>::new(), 0, 1, 1) + .await; + assert_eq!(written, 0); + assert_eq!(err.expect("reader count mismatch should fail").kind(), ErrorKind::InvalidInput); + + let readers: Vec>>>> = vec![None, None, None]; + let (written, err) = erasure.decode(&mut output, readers, usize::MAX, 1, usize::MAX).await; + assert_eq!(written, 0); + assert_eq!(err.expect("offset overflow should fail").kind(), ErrorKind::InvalidInput); + + let readers: Vec>>>> = vec![None, None, None]; + let (written, err) = erasure.decode(&mut output, readers, 2, 8, 9).await; + assert_eq!(written, 0); + assert_eq!(err.expect("range beyond total length should fail").kind(), ErrorKind::InvalidInput); + } + + #[tokio::test] + async fn test_erasure_decode_with_read_costs_restores_missing_data_shard_range() { + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 2; + const BLOCK_SIZE: usize = 64; + + let data: Vec = (0..BLOCK_SIZE as u8).collect(); + let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE); + let shard_size = erasure.shard_size(); + let encoded = erasure.encode_data(&data).expect("encode should succeed"); + let readers = vec![ + None, + Some(BitrotReader::new( + Cursor::new(encoded[1].to_vec()), + shard_size, + HashAlgorithm::None, + false, + )), + Some(BitrotReader::new( + Cursor::new(encoded[DATA_SHARDS].to_vec()), + shard_size, + HashAlgorithm::None, + false, + )), + Some(BitrotReader::new( + Cursor::new(encoded[DATA_SHARDS + 1].to_vec()), + shard_size, + HashAlgorithm::None, + false, + )), + ]; + let read_costs = vec![ + ShardReadCost::Local, + ShardReadCost::SameNode, + ShardReadCost::Remote, + ShardReadCost::Unknown, + ]; + + let mut output = Vec::new(); + let (written, err) = erasure + .decode_with_read_costs(&mut output, readers, 5, 37, data.len(), read_costs) + .await; + + assert!(err.is_none(), "missing data shard should reconstruct with parity: {err:?}"); + assert_eq!(written, 37); + assert_eq!(output, data[5..42]); + } + /// Regression for upstream issue #2716: ranged GETs going through /// `Erasure::decode` must return the requested byte range without /// panicking or truncating, including when the range starts at a @@ -3007,6 +3101,58 @@ mod tests { rustfs_io_metrics::set_get_stage_metrics_enabled(false); } + #[tokio::test] + #[serial_test::serial] + async fn parallel_reader_records_metrics_for_observe_and_locality_policy_modes() { + const NUM_SHARDS: usize = 1; + const BLOCK_SIZE: usize = 64; + const DATA_SHARDS: usize = 4; + const PARITY_SHARDS: usize = 2; + const SHARD_SIZE: usize = BLOCK_SIZE / DATA_SHARDS; + + for (mode, expected_data_slots) in [("off", vec![0, 1, 2, 3]), ("on", vec![2, 3, 4, 5])] { + temp_env::async_with_vars( + [ + (ENV_RUSTFS_SHARD_LOCALITY_SCHEDULING, Some(mode)), + (ENV_RUSTFS_GET_SHARD_LOCALITY_PREFERENCE_ENABLE, None::<&str>), + ], + async { + rustfs_io_metrics::set_get_stage_metrics_enabled(true); + let hash_algo = HashAlgorithm::HighwayHash256; + let readers = + make_test_readers(DATA_SHARDS + PARITY_SHARDS, SHARD_SIZE, NUM_SHARDS, &hash_algo, &[], &[]).await; + let read_costs = vec![ + ShardReadCost::Remote, + ShardReadCost::Remote, + ShardReadCost::Local, + ShardReadCost::SameNode, + ShardReadCost::Local, + ShardReadCost::SameNode, + ]; + let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE); + let mut parallel_reader = ParallelReader::new_with_metrics_path_and_read_costs( + readers, + erasure, + 0, + NUM_SHARDS * BLOCK_SIZE, + Some(GET_OBJECT_PATH_LEGACY_DUPLEX), + read_costs, + ); + + let (bufs, errs) = parallel_reader.read().await; + + assert_eq!(parallel_reader.metrics_path, Some(GET_OBJECT_PATH_LEGACY_DUPLEX)); + assert!(errs.iter().all(Option::is_none)); + for index in expected_data_slots { + assert_eq!(bufs[index].as_deref(), Some(&[(index % 256) as u8; SHARD_SIZE][..])); + } + rustfs_io_metrics::set_get_stage_metrics_enabled(false); + }, + ) + .await; + } + } + #[tokio::test] #[serial_test::serial] async fn test_parallel_reader_local_first_avoids_remote_when_local_quorum_exists() { @@ -3342,6 +3488,101 @@ mod tests { assert_eq!(DATA_SHARDS, bufs.iter().filter(|buf| buf.is_some()).count()); } + #[tokio::test] + async fn test_parallel_reader_schedules_extra_parity_for_reconstruction_verification() { + const BLOCK_SIZE: usize = 64; + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 2; + const SHARD_SIZE: usize = BLOCK_SIZE / DATA_SHARDS; + + let hash_algo = HashAlgorithm::None; + let readers = vec![ + None, + Some(BitrotReader::new( + TestShardReader::Ready(Cursor::new(vec![1_u8; SHARD_SIZE])), + SHARD_SIZE, + hash_algo.clone(), + false, + )), + Some(BitrotReader::new( + TestShardReader::Ready(Cursor::new(vec![2_u8; SHARD_SIZE])), + SHARD_SIZE, + hash_algo.clone(), + false, + )), + Some(BitrotReader::new( + TestShardReader::Ready(Cursor::new(vec![3_u8; SHARD_SIZE])), + SHARD_SIZE, + hash_algo, + false, + )), + ]; + + let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE); + let mut parallel_reader = + ParallelReader::new_with_metrics_path_and_reconstruction_verification(readers, erasure, 0, BLOCK_SIZE, None); + + let (bufs, errs) = parallel_reader.read().await; + + assert!(errs[0].is_none() || matches!(&errs[0], Some(DiskError::FileNotFound))); + assert!(bufs[0].is_none()); + assert_eq!(3, bufs.iter().filter(|buf| buf.is_some()).count()); + assert!(bufs[1].is_some()); + assert!(bufs[2].is_some()); + assert!(bufs[3].is_some()); + } + + #[tokio::test] + #[serial_test::serial] + async fn test_parallel_reader_records_metrics_for_success_missing_error_and_timeout() { + const NUM_SHARDS: usize = 1; + const BLOCK_SIZE: usize = 64; + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 2; + const SHARD_SIZE: usize = BLOCK_SIZE / DATA_SHARDS; + + rustfs_io_metrics::set_get_stage_metrics_enabled(true); + let hash_algo = HashAlgorithm::None; + let readers = vec![ + Some(BitrotReader::new(TestShardReader::Pending, SHARD_SIZE, hash_algo.clone(), false)), + Some(BitrotReader::new(TestShardReader::TimedOut, SHARD_SIZE, hash_algo.clone(), false)), + Some(BitrotReader::new( + TestShardReader::Ready(Cursor::new(vec![2_u8; SHARD_SIZE * NUM_SHARDS])), + SHARD_SIZE, + hash_algo, + false, + )), + None, + ]; + let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE); + let mut parallel_reader = ParallelReader::new_with_metrics_path_read_costs_timeout_and_reconstruction_verification( + readers, + erasure, + 0, + NUM_SHARDS * BLOCK_SIZE, + Some(GET_OBJECT_PATH_LEGACY_DUPLEX), + vec![ + ShardReadCost::Local, + ShardReadCost::SameNode, + ShardReadCost::Remote, + ShardReadCost::Unknown, + ], + Duration::from_millis(20), + false, + ); + + let started = std::time::Instant::now(); + let (bufs, errs) = parallel_reader.read().await; + + rustfs_io_metrics::set_get_stage_metrics_enabled(false); + assert!(started.elapsed() < Duration::from_secs(1)); + assert_eq!(parallel_reader.metrics_path, Some(GET_OBJECT_PATH_LEGACY_DUPLEX)); + assert!(matches!(&errs[0], Some(DiskError::Io(err)) if err.kind() == ErrorKind::TimedOut)); + assert!(matches!(&errs[1], Some(DiskError::Io(err)) if err.kind() == ErrorKind::TimedOut)); + assert!(matches!(&errs[3], Some(Error::FileNotFound))); + assert_eq!(bufs[2].as_deref(), Some(&[2_u8; SHARD_SIZE][..])); + } + #[tokio::test] async fn test_parallel_reader_uses_parity_without_waiting_for_pending_shard() { const NUM_SHARDS: usize = 1; @@ -3383,6 +3624,39 @@ mod tests { assert_eq!(DATA_SHARDS, bufs.iter().filter(|buf| buf.is_some()).count()); } + #[tokio::test] + async fn test_parallel_reader_drains_completed_shards_after_quorum() { + const NUM_SHARDS: usize = 1; + const BLOCK_SIZE: usize = 64; + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 3; + const SHARD_SIZE: usize = BLOCK_SIZE / DATA_SHARDS; + + let ready_at = TokioInstant::now() + Duration::from_millis(250); + let readers = (0..DATA_SHARDS + PARITY_SHARDS) + .map(|index| { + Some(BitrotReader::new( + TestShardReader::ReadyAt { + cursor: Cursor::new(vec![index as u8; SHARD_SIZE * NUM_SHARDS]), + ready_at, + sleep: None, + }, + SHARD_SIZE, + HashAlgorithm::None, + false, + )) + }) + .collect(); + let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE); + let mut parallel_reader = + ParallelReader::new_with_read_timeout(readers, erasure, 0, NUM_SHARDS * BLOCK_SIZE, Duration::from_secs(2)); + + let (bufs, errs) = parallel_reader.read().await; + + assert!(errs.iter().all(Option::is_none)); + assert!(bufs.iter().filter(|buf| buf.is_some()).count() >= DATA_SHARDS); + } + #[tokio::test] async fn test_parallel_reader_retires_partially_read_shard_after_quorum() { const NUM_SHARDS: usize = 2; @@ -3484,6 +3758,19 @@ mod tests { assert_eq!(shard_read_hedge_delay(Duration::ZERO), None); assert_eq!(shard_read_hedge_delay(Duration::from_millis(50)), Some(Duration::from_millis(50))); assert_eq!(shard_read_hedge_delay(Duration::from_secs(60)), Some(Duration::from_millis(100))); + + temp_env::with_var(ENV_RUSTFS_GET_DECODE_STRIPE_PREFETCH_COUNT, None::<&str>, || { + assert_eq!(get_decode_stripe_prefetch_count(), DEFAULT_RUSTFS_GET_DECODE_STRIPE_PREFETCH_COUNT); + }); + temp_env::with_var(ENV_RUSTFS_GET_DECODE_STRIPE_PREFETCH_COUNT, Some("3"), || { + assert_eq!(get_decode_stripe_prefetch_count(), 3); + }); + temp_env::with_var(ENV_RUSTFS_GET_BITROT_DECODE_OVERLAP_ENABLE, None::<&str>, || { + assert_eq!(is_bitrot_decode_overlap_enabled(), DEFAULT_RUSTFS_GET_BITROT_DECODE_OVERLAP_ENABLE); + }); + temp_env::with_var(ENV_RUSTFS_GET_BITROT_DECODE_OVERLAP_ENABLE, Some("true"), || { + assert!(is_bitrot_decode_overlap_enabled()); + }); } async fn create_reader( diff --git a/crates/ecstore/src/erasure/coding/decode_reader.rs b/crates/ecstore/src/erasure/coding/decode_reader.rs index 68f06ac0d..91d4c6479 100644 --- a/crates/ecstore/src/erasure/coding/decode_reader.rs +++ b/crates/ecstore/src/erasure/coding/decode_reader.rs @@ -789,7 +789,7 @@ fn emit_data_shards(state: &StripeReadState, data_shards: usize, block_size: usi fn reserve_output_capacity(output: &mut Vec, target_capacity: usize) { if output.capacity() < target_capacity { - output.reserve(target_capacity - output.capacity()); + output.reserve(target_capacity.saturating_sub(output.len())); } } @@ -822,14 +822,14 @@ fn emit_data_shards_into( mod tests { use super::*; use crate::erasure::codec::bridge::{ - CodecStreamingDecodeEngine, ErasureDecodeEngine, LegacyEcDecodeEngine, RustfsCodecDecodeEngine, + CodecStreamingDecodeEngine, DecodeWorkspace, ErasureDecodeEngine, LegacyEcDecodeEngine, RustfsCodecDecodeEngine, }; use crate::erasure::coding::decode::ParallelReader; use crate::erasure::coding::{BitrotReader, BitrotWriter, Erasure}; use crate::set_disk::shard_source::{ShardSlot, StripeReadState}; use rustfs_utils::HashAlgorithm; use std::collections::VecDeque; - use std::future::pending; + use std::future::{pending, poll_fn}; use std::io::Cursor; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -855,6 +855,42 @@ mod tests { dropped: Arc, } + enum PollStep { + Data(Vec), + Empty, + Error, + Pending, + } + + struct ScriptedAsyncReader { + steps: VecDeque, + } + + impl ScriptedAsyncReader { + fn new(steps: Vec) -> Self { + Self { steps: steps.into() } + } + } + + impl AsyncRead for ScriptedAsyncReader { + fn poll_read(mut self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &mut ReadBuf<'_>) -> Poll> { + match self.steps.pop_front() { + Some(PollStep::Data(data)) => { + let copy_len = data.len().min(buf.remaining()); + buf.put_slice(&data[..copy_len]); + Poll::Ready(Ok(())) + } + Some(PollStep::Empty) => Poll::Ready(Ok(())), + Some(PollStep::Error) => Poll::Ready(Err(io::Error::other("scripted read failure"))), + Some(PollStep::Pending) => { + cx.waker().wake_by_ref(); + Poll::Pending + } + None => Poll::Ready(Ok(())), + } + } + } + impl Drop for BlockingSourceDropGuard { fn drop(&mut self) { self.dropped.fetch_add(1, Ordering::SeqCst); @@ -885,6 +921,62 @@ mod tests { } } + #[derive(Clone)] + struct NoopDecodeEngine { + data_shards: usize, + block_size: usize, + } + + struct NoopDecodeWorkspace { + shard_len: usize, + } + + impl DecodeWorkspace for NoopDecodeWorkspace { + fn shard_len(&self) -> usize { + self.shard_len + } + } + + impl ErasureDecodeEngine for NoopDecodeEngine { + type Workspace = NoopDecodeWorkspace; + + fn data_shards(&self) -> usize { + self.data_shards + } + + fn parity_shards(&self) -> usize { + 0 + } + + fn block_size(&self) -> usize { + self.block_size + } + + fn engine_name(&self) -> &'static str { + "noop" + } + + fn supports_progressive_decode(&self) -> bool { + false + } + + fn supports_aligned_shards(&self) -> bool { + false + } + + fn prepare_workspace(&self, shard_len: usize) -> io::Result { + Ok(NoopDecodeWorkspace { shard_len }) + } + + fn reconstruct_into( + &self, + _shards: &mut [Option>], + _workspace: &mut Self::Workspace, + ) -> io::Result<&'static str> { + Ok("noop_called") + } + } + fn source_from_data(erasure: &Erasure, data: &[u8], missing_indexes: &[usize]) -> VecStripeSource { let read_quorum = erasure.data_shards; let stripes = data @@ -998,6 +1090,309 @@ mod tests { }); } + #[test] + fn erasure_decode_reader_rejects_invalid_engine_shape() { + let source = VecStripeSource { + stripes: VecDeque::new(), + read_quorum: 1, + read_count: None, + }; + let err = match ErasureDecodeReader::new( + source, + NoopDecodeEngine { + data_shards: 0, + block_size: 16, + }, + 1, + ) { + Ok(_) => panic!("zero data shard engine must be rejected"), + Err(err) => err, + }; + assert_eq!(err.kind(), ErrorKind::InvalidInput); + + let source = VecStripeSource { + stripes: VecDeque::new(), + read_quorum: 1, + read_count: None, + }; + let err = match ErasureDecodeReader::new_with_metrics_path( + source, + NoopDecodeEngine { + data_shards: 1, + block_size: 0, + }, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + ) { + Ok(_) => panic!("zero block size engine must be rejected"), + Err(err) => err, + }; + assert_eq!(err.kind(), ErrorKind::InvalidInput); + } + + #[test] + fn erasure_decode_reader_reusable_buffer_bounds_and_missing_worker_parts_fail_closed() { + let erasure = Erasure::new(2, 1, 16); + let source = source_from_data(&erasure, b"fill worker missing fields", &[]); + let engine = LegacyEcDecodeEngine::new(erasure.clone()); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::SingleInFlight, + ) + .expect("reader should be constructed"); + + reader.push_reusable_output_buf(Vec::new()); + assert!(reader.reusable_output_bufs.is_empty()); + for _ in 0..reader.max_reusable_output_bufs() + 2 { + reader.push_reusable_output_buf(Vec::with_capacity(8)); + } + assert_eq!(reader.reusable_output_bufs.len(), reader.max_reusable_output_bufs()); + + reader.source = None; + let err = match reader.fill_worker_tx() { + Ok(_) => panic!("missing source must fail closed"), + Err(err) => err, + }; + assert_eq!(err.kind(), ErrorKind::BrokenPipe); + + let source = source_from_data(&erasure, b"fill worker missing engine", &[]); + let engine = LegacyEcDecodeEngine::new(erasure.clone()); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::SingleInFlight, + ) + .expect("reader should be constructed"); + reader.engine = None; + let err = match reader.fill_worker_tx() { + Ok(_) => panic!("missing engine must fail closed"), + Err(err) => err, + }; + assert_eq!(err.kind(), ErrorKind::BrokenPipe); + assert!(reader.source.is_some()); + + let source = source_from_data(&erasure, b"fill worker missing workspace", &[]); + let engine = LegacyEcDecodeEngine::new(erasure); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::SingleInFlight, + ) + .expect("reader should be constructed"); + reader.workspace = None; + let err = match reader.fill_worker_tx() { + Ok(_) => panic!("missing workspace must fail closed"), + Err(err) => err, + }; + assert_eq!(err.kind(), ErrorKind::BrokenPipe); + assert!(reader.source.is_some()); + assert!(reader.engine.is_some()); + } + + #[tokio::test] + async fn erasure_decode_reader_poll_fill_result_rejects_cancelled_and_empty_fill() { + let erasure = Erasure::new(2, 1, 16); + let source = source_from_data(&erasure, b"cancelled fill", &[]); + let engine = LegacyEcDecodeEngine::new(erasure.clone()); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::SingleInFlight, + ) + .expect("reader should be constructed"); + let (_sender, receiver) = oneshot::channel(); + drop(_sender); + reader.fill = Some(receiver); + + let err = poll_fn(|cx| reader.poll_fill_result(cx)) + .await + .expect_err("cancelled fill result must fail"); + assert_eq!(err.kind(), ErrorKind::Other); + + let source = source_from_data(&erasure, b"empty fill", &[]); + let engine = LegacyEcDecodeEngine::new(erasure); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::SingleInFlight, + ) + .expect("reader should be constructed"); + let (sender, receiver) = oneshot::channel(); + assert!( + sender + .send(FillResult { + result: Ok(Some(Vec::new())), + queued_buffers: VecDeque::new(), + reusable_buffers: Vec::new(), + deferred_error: None, + }) + .is_ok(), + "test fill result should send" + ); + reader.fill = Some(receiver); + + let err = poll_fn(|cx| reader.poll_fill_result(cx)) + .await + .expect_err("empty buffer with remaining bytes must fail"); + assert_eq!(err.kind(), ErrorKind::Other); + } + + #[tokio::test] + async fn erasure_decode_reader_poll_fill_result_fails_when_request_queue_is_full() { + let erasure = Erasure::new(2, 1, 16); + let source = source_from_data(&erasure, b"queue full", &[]); + let engine = LegacyEcDecodeEngine::new(erasure); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 1, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::SingleInFlight, + ) + .expect("reader should be constructed"); + let (tx, rx) = mpsc::channel(1); + let (response, _receiver) = oneshot::channel(); + assert!( + tx.try_send(FillRequest { + remaining: 1, + reusable_buffers: Vec::new(), + response, + }) + .is_ok(), + "test fill request should occupy the bounded queue" + ); + reader.worker = Some(FillWorker { + tx, + task: tokio::spawn(async move { + pending::<()>().await; + drop(rx); + }), + }); + reader.reusable_output_bufs.push(Vec::with_capacity(8)); + + let err = poll_fn(|cx| reader.poll_fill_result(cx)) + .await + .expect_err("full fill request queue must fail closed"); + + assert_eq!(err.kind(), ErrorKind::BrokenPipe); + assert_eq!(reader.reusable_output_bufs.len(), 1); + } + + #[tokio::test] + async fn erasure_decode_reader_prefetch_queues_fill_when_output_is_not_drained() { + let erasure = Erasure::new(2, 1, 16); + let source = source_from_data(&erasure, b"queued prefetch", &[]); + let engine = LegacyEcDecodeEngine::new(erasure); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 4, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::DualInFlight, + ) + .expect("reader should be constructed"); + let (sender, receiver) = oneshot::channel(); + assert!( + sender + .send(FillResult { + result: Ok(Some(vec![3, 5, 8])), + queued_buffers: VecDeque::new(), + reusable_buffers: Vec::new(), + deferred_error: None, + }) + .is_ok(), + "ready fill result should send" + ); + reader.fill = Some(receiver); + reader.output_buf = vec![1, 2]; + reader.output_pos = 1; + + poll_fn(|cx| reader.poll_prefetch(cx)) + .await + .expect("ready prefetch should be queued while output remains"); + + assert_eq!(reader.output_buf, vec![1, 2]); + assert_eq!(reader.output_pos, 1); + assert_eq!(reader.prefetched_bufs.pop_front(), Some(vec![3, 5, 8])); + + reader.prefetched_bufs.push_back(vec![3, 5, 8]); + reader.output_pos = reader.output_buf.len(); + let mut output = [0u8; 3]; + reader + .read_exact(&mut output) + .await + .expect("queued prefetch should become reader output after the old buffer drains"); + assert_eq!(output, [3, 5, 8]); + } + + #[tokio::test] + async fn erasure_decode_reader_prefetch_defers_error_until_output_drains() { + let erasure = Erasure::new(2, 1, 16); + let source = source_from_data(&erasure, b"deferred prefetch error", &[]); + let engine = LegacyEcDecodeEngine::new(erasure); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + 4, + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::DualInFlight, + ) + .expect("reader should be constructed"); + let (sender, receiver) = oneshot::channel(); + assert!( + sender + .send(FillResult { + result: Err(io::Error::new(ErrorKind::UnexpectedEof, "deferred fill error")), + queued_buffers: VecDeque::new(), + reusable_buffers: Vec::new(), + deferred_error: None, + }) + .is_ok(), + "ready fill error should send" + ); + reader.fill = Some(receiver); + reader.output_buf = vec![1, 2]; + reader.output_pos = 1; + + poll_fn(|cx| reader.poll_prefetch(cx)) + .await + .expect("prefetch error should be deferred while output remains"); + + assert_eq!( + reader + .prefetch_error + .as_ref() + .expect("prefetch error should be retained") + .kind(), + ErrorKind::UnexpectedEof + ); + } + + #[test] + fn noop_decode_engine_test_methods_report_static_shape() { + let engine = NoopDecodeEngine { + data_shards: 3, + block_size: 96, + }; + let workspace = engine.prepare_workspace(32).expect("workspace should be created"); + + assert_eq!(workspace.shard_len(), 32); + assert_eq!(engine.parity_shards(), 0); + assert!(!engine.supports_progressive_decode()); + assert!(!engine.supports_aligned_shards()); + } + #[test] #[serial_test::serial] fn erasure_decode_reader_caches_stage_metrics_enabled_at_construction() { @@ -1033,6 +1428,102 @@ mod tests { rustfs_io_metrics::set_get_stage_metrics_enabled(false); } + #[tokio::test] + #[serial_test::serial] + async fn erasure_decode_reader_records_metrics_while_copying_output() { + let erasure = Erasure::new(4, 2, 16); + let data = (0..48u8).collect::>(); + let read_count = Arc::new(AtomicUsize::new(0)); + rustfs_io_metrics::set_get_stage_metrics_enabled(true); + + let mut source = source_from_data(&erasure, &data, &[]); + source.read_count = Some(Arc::clone(&read_count)); + let engine = LegacyEcDecodeEngine::new(erasure); + let mut reader = ErasureDecodeReader::new_with_fill_policy( + source, + engine, + data.len(), + GET_OBJECT_PATH_CODEC_STREAMING, + FillPolicy::DualInFlight, + ) + .expect("reader should be constructed"); + assert!(reader.stage_metrics_enabled); + + let mut first = [0u8; 1]; + let read = reader + .read(&mut first) + .await + .expect("metrics-enabled reader should produce first byte"); + timeout(Duration::from_secs(1), async { + while read_count.load(Ordering::SeqCst) < 3 { + yield_now().await; + } + }) + .await + .expect("metrics-enabled dual inflight reader should prefetch future stripes"); + + rustfs_io_metrics::set_get_stage_metrics_enabled(false); + assert_eq!(read, 1); + assert_eq!(first[0], data[0]); + } + + #[tokio::test] + #[serial_test::serial] + async fn sync_erasure_decode_reader_records_metric_poll_outcomes() { + rustfs_io_metrics::set_get_stage_metrics_enabled(true); + let mut reader = SyncErasureDecodeReader::new_with_metrics_path( + ScriptedAsyncReader::new(vec![ + PollStep::Pending, + PollStep::Data(vec![3, 5]), + PollStep::Empty, + PollStep::Error, + ]), + GET_OBJECT_PATH_CODEC_STREAMING, + ); + assert!(reader.stage_metrics_enabled); + let mut output = [0u8; 4]; + + let read = reader + .read(&mut output) + .await + .expect("pending reader should wake and then return data"); + assert_eq!(read, 2); + assert_eq!(&output[..read], &[3, 5]); + + let read = reader.read(&mut output).await.expect("empty ready poll should return EOF"); + assert_eq!(read, 0); + + let err = reader + .read(&mut output) + .await + .expect_err("scripted read error should surface"); + assert_eq!(err.kind(), ErrorKind::Other); + rustfs_io_metrics::set_get_stage_metrics_enabled(false); + } + + #[tokio::test] + #[serial_test::serial] + async fn sync_erasure_decode_reader_fails_closed_on_poisoned_lock() { + let mut reader = SyncErasureDecodeReader::new(Cursor::new(vec![1u8])); + let previous_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(|_| {})); + let poison_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let _guard = reader.inner.lock().expect("lock should be acquired before poison"); + panic!("poison sync reader lock"); + })); + std::panic::set_hook(previous_hook); + assert!(poison_result.is_err(), "test setup should poison the reader lock"); + let mut output = [0u8; 1]; + + let err = reader + .read(&mut output) + .await + .expect_err("poisoned sync reader lock must fail closed"); + + assert_eq!(err.kind(), ErrorKind::Other); + assert!(err.to_string().contains("lock poisoned")); + } + #[tokio::test] async fn erasure_decode_reader_reads_single_stripe() { let erasure = Erasure::new(4, 2, 64); @@ -1189,6 +1680,84 @@ mod tests { assert_eq!(single, dual); } + #[tokio::test] + async fn run_fill_request_dual_inflight_returns_deferred_eof_and_decode_errors() { + let erasure = Erasure::new(4, 2, 16); + let first = (0..16u8).collect::>(); + let first_state = source_from_data(&erasure, &first, &[]) + .stripes + .pop_front() + .expect("first stripe should exist"); + let mut source = VecStripeSource { + stripes: VecDeque::from([first_state, StripeReadState::new(Vec::new(), erasure.data_shards)]), + read_quorum: erasure.data_shards, + read_count: None, + }; + let engine = LegacyEcDecodeEngine::new(erasure.clone()); + let mut workspace = engine + .prepare_workspace(erasure.shard_size()) + .expect("workspace should be prepared"); + + let result = run_fill_request(FillRequestWork { + source: &mut source, + engine: &engine, + workspace: &mut workspace, + fill_policy: FillPolicy::DualInFlight, + metrics_path: GET_OBJECT_PATH_CODEC_STREAMING, + stage_metrics_enabled: false, + remaining: first.len() + 1, + reusable_buffers: vec![Vec::new(), Vec::new()], + }) + .await; + + assert_eq!(result.result.as_ref().expect("first stripe should decode").as_deref(), Some(&first[..])); + assert_eq!( + result + .deferred_error + .as_ref() + .expect("second empty stripe should defer LessData") + .kind(), + ErrorKind::Other + ); + + let first_state = source_from_data(&erasure, &first, &[]) + .stripes + .pop_front() + .expect("first stripe should exist"); + let mut source = VecStripeSource { + stripes: VecDeque::from([ + first_state, + StripeReadState::new(vec![ShardSlot::data(0, vec![1])], erasure.data_shards), + ]), + read_quorum: erasure.data_shards, + read_count: None, + }; + let engine = LegacyEcDecodeEngine::new(erasure); + let mut workspace = engine.prepare_workspace(4).expect("workspace should be prepared"); + + let result = run_fill_request(FillRequestWork { + source: &mut source, + engine: &engine, + workspace: &mut workspace, + fill_policy: FillPolicy::DualInFlight, + metrics_path: GET_OBJECT_PATH_CODEC_STREAMING, + stage_metrics_enabled: false, + remaining: first.len() + 1, + reusable_buffers: vec![Vec::new(), Vec::new()], + }) + .await; + + assert!(result.result.expect("first stripe should decode").is_some()); + assert_eq!( + result + .deferred_error + .as_ref() + .expect("second quorum failure should be deferred") + .kind(), + ErrorKind::Other + ); + } + #[tokio::test] async fn erasure_decode_reader_defers_short_read_error_until_buffer_drains() { let erasure = Erasure::new(4, 2, 32); @@ -1474,6 +2043,45 @@ mod tests { assert_eq!(output, b"abcde"); } + #[test] + fn decode_stripe_into_rejects_missing_reconstructed_data_shards() { + let engine = NoopDecodeEngine { + data_shards: 2, + block_size: 8, + }; + let mut workspace = engine.prepare_workspace(4).expect("workspace should be prepared"); + let mut output = Vec::with_capacity(1); + let short_state = StripeReadState::new(vec![ShardSlot::data(0, vec![1, 2, 3, 4])], 1); + + let err = decode_stripe_into( + GET_OBJECT_PATH_CODEC_STREAMING, + false, + &engine, + &mut workspace, + short_state, + 8, + &mut output, + ) + .expect_err("decoded stripe shorter than data shard count must fail"); + assert_eq!(err.kind(), ErrorKind::UnexpectedEof); + + let missing_state = StripeReadState::from_parts(vec![None, Some(vec![5, 6, 7, 8])], Vec::new(), 1); + let err = decode_stripe_into( + GET_OBJECT_PATH_CODEC_STREAMING, + false, + &engine, + &mut workspace, + missing_state, + 8, + &mut output, + ) + .expect_err("missing reconstructed data shard must fail"); + assert_eq!(err.kind(), ErrorKind::UnexpectedEof); + + reserve_output_capacity(&mut output, 32); + assert!(output.capacity() >= 32); + } + #[tokio::test] async fn erasure_decode_reader_reports_short_source() { let erasure = Erasure::new(4, 2, 32); diff --git a/crates/ecstore/src/erasure/coding/encode.rs b/crates/ecstore/src/erasure/coding/encode.rs index 260d6fd6c..25d0172c4 100644 --- a/crates/ecstore/src/erasure/coding/encode.rs +++ b/crates/ecstore/src/erasure/coding/encode.rs @@ -42,6 +42,7 @@ const DEFAULT_RUSTFS_ERASURE_ENCODE_BYTESMUT_INGEST: bool = false; /// Read once at first use via `OnceLock` to avoid per-encode syscall. static CACHED_MAX_INFLIGHT_BYTES: std::sync::OnceLock = std::sync::OnceLock::new(); static CACHED_BATCH_BLOCKS: std::sync::OnceLock = std::sync::OnceLock::new(); +#[cfg(not(test))] static CACHED_BYTESMUT_INGEST: std::sync::OnceLock = std::sync::OnceLock::new(); #[inline(always)] @@ -83,6 +84,12 @@ fn erasure_encode_max_inflight_bytes() -> usize { } fn use_bytesmut_ingest() -> bool { + #[cfg(test)] + { + rustfs_utils::get_env_bool(ENV_RUSTFS_ERASURE_ENCODE_BYTESMUT_INGEST, DEFAULT_RUSTFS_ERASURE_ENCODE_BYTESMUT_INGEST) + } + + #[cfg(not(test))] *CACHED_BYTESMUT_INGEST.get_or_init(|| { rustfs_utils::get_env_bool(ENV_RUSTFS_ERASURE_ENCODE_BYTESMUT_INGEST, DEFAULT_RUSTFS_ERASURE_ENCODE_BYTESMUT_INGEST) }) @@ -230,35 +237,24 @@ impl<'a> MultiWriter<'a> { return Ok(()); } - if let Some(write_err) = reduce_write_quorum_errs(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum) { - let summary = build_write_quorum_failure_summary(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum); - let summary_text = format_write_quorum_failure(&summary); - runtime_sources::record_erasure_write_quorum_failure("write", quorum_dominant_error_metric_label(&summary)); - error!( - required = summary.required, - achieved = summary.achieved, - failed = summary.failed, - total = summary.total, - offline_disks = summary.offline_disks, - retryable_failures = summary.retryable_failures, - dominant_error = summary.dominant_error_label, - returned_error = %write_err, - errs = ?self.errs, - "Erasure encode write quorum unavailable: {summary_text}" - ); - return Err(std::io::Error::other(format!("Failed to write data: {summary_text}"))); - } - + let write_err = + reduce_write_quorum_errs(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum).unwrap_or(Error::ErasureWriteQuorum); let summary = build_write_quorum_failure_summary(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum); - Err(std::io::Error::other(format!( - "Failed to write data: {}: {}", - format_write_quorum_failure(&summary), - self.errs - .iter() - .map(|e| e.as_ref().map_or_else(|| "".to_string(), |e| e.to_string())) - .collect::>() - .join(", ") - ))) + let summary_text = format_write_quorum_failure(&summary); + runtime_sources::record_erasure_write_quorum_failure("write", quorum_dominant_error_metric_label(&summary)); + error!( + required = summary.required, + achieved = summary.achieved, + failed = summary.failed, + total = summary.total, + offline_disks = summary.offline_disks, + retryable_failures = summary.retryable_failures, + dominant_error = summary.dominant_error_label, + returned_error = %write_err, + errs = ?self.errs, + "Erasure encode write quorum unavailable: {summary_text}" + ); + Err(std::io::Error::other(format!("Failed to write data: {summary_text}"))) } async fn shutdown_writer(writer_opt: &mut Option, err: &mut Option) { @@ -296,35 +292,24 @@ impl<'a> MultiWriter<'a> { return Ok(()); } - if let Some(write_err) = reduce_write_quorum_errs(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum) { - let summary = build_write_quorum_failure_summary(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum); - let summary_text = format_write_quorum_failure(&summary); - runtime_sources::record_erasure_write_quorum_failure("shutdown", quorum_dominant_error_metric_label(&summary)); - error!( - required = summary.required, - achieved = summary.achieved, - failed = summary.failed, - total = summary.total, - offline_disks = summary.offline_disks, - retryable_failures = summary.retryable_failures, - dominant_error = summary.dominant_error_label, - returned_error = %write_err, - errs = ?self.errs, - "Erasure encode shutdown quorum unavailable: {summary_text}" - ); - return Err(std::io::Error::other(format!("Failed to shutdown writers: {summary_text}"))); - } - + let write_err = + reduce_write_quorum_errs(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum).unwrap_or(Error::ErasureWriteQuorum); let summary = build_write_quorum_failure_summary(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum); - Err(std::io::Error::other(format!( - "Failed to shutdown writers: {}: {}", - format_write_quorum_failure(&summary), - self.errs - .iter() - .map(|e| e.as_ref().map_or_else(|| "".to_string(), |e| e.to_string())) - .collect::>() - .join(", ") - ))) + let summary_text = format_write_quorum_failure(&summary); + runtime_sources::record_erasure_write_quorum_failure("shutdown", quorum_dominant_error_metric_label(&summary)); + error!( + required = summary.required, + achieved = summary.achieved, + failed = summary.failed, + total = summary.total, + offline_disks = summary.offline_disks, + retryable_failures = summary.retryable_failures, + dominant_error = summary.dominant_error_label, + returned_error = %write_err, + errs = ?self.errs, + "Erasure encode shutdown quorum unavailable: {summary_text}" + ); + Err(std::io::Error::other(format!("Failed to shutdown writers: {summary_text}"))) } } @@ -753,7 +738,7 @@ mod tests { use std::pin::Pin; use std::sync::{Arc, Mutex}; use std::task::{Context, Poll}; - use tokio::io::AsyncWrite; + use tokio::io::{AsyncWrite, AsyncWriteExt}; #[derive(Clone, Default)] struct DeferredCommitWriter { @@ -849,6 +834,30 @@ mod tests { BitrotWriterWrapper::new(CustomWriter::new_tokio_writer(writer), shard_size, HashAlgorithm::HighwayHash256S) } + #[tokio::test] + async fn helper_writers_cover_flush_and_shutdown_paths() { + let mut failing_write = FailingWriteWriter; + failing_write.flush().await.expect("failing-write flush should succeed"); + failing_write.shutdown().await.expect("failing-write shutdown should succeed"); + + let mut short_write = ShortWriteWriter; + let written = short_write + .write(b"short") + .await + .expect("short-write helper should report a partial write"); + assert_eq!(written, 4); + short_write.flush().await.expect("short-write flush should succeed"); + short_write.shutdown().await.expect("short-write shutdown should succeed"); + + let mut shutdown_fail = ShutdownFailWriter::default(); + shutdown_fail.flush().await.expect("shutdown-fail flush should succeed"); + let err = shutdown_fail + .shutdown() + .await + .expect_err("shutdown-fail writer should reject shutdown"); + assert_eq!(err.to_string(), "injected shutdown failure"); + } + #[tokio::test] async fn multi_writer_short_write_fails_before_shutdown() { let mut writers = vec![Some(bitrot_writer(ShortWriteWriter, 16))]; @@ -864,6 +873,61 @@ mod tests { assert!(writers[0].is_none(), "short-write shard must be removed before commit"); } + #[tokio::test] + async fn multi_writer_reports_fallback_summary_when_only_offline_writers_remain() { + let mut writers = vec![None, None]; + let err = { + let mut writer = MultiWriter::new(&mut writers, 1); + writer + .write(vec![Bytes::from_static(b"offline-a"), Bytes::from_static(b"offline-b")]) + .await + .expect_err("offline writers cannot satisfy write quorum") + }; + + let err = err.to_string(); + assert!(err.contains("Failed to write data")); + assert!(err.contains("offline-disks=2/2")); + assert!(err.contains("required=1")); + + let shutdown_err = { + let mut writer = MultiWriter::new(&mut writers, 1); + writer + .shutdown() + .await + .expect_err("offline writers cannot satisfy shutdown quorum") + }; + + let shutdown_err = shutdown_err.to_string(); + assert!(shutdown_err.contains("Failed to shutdown writers")); + assert!(shutdown_err.contains("offline-disks=2/2")); + assert!(shutdown_err.contains("required=1")); + } + + #[tokio::test] + async fn multi_writer_reports_quorum_failure_when_quorum_exceeds_writer_count() { + let committed = Arc::new(Mutex::new(Vec::new())); + let mut writers = vec![Some(bitrot_writer(DeferredCommitWriter::new(committed), 16))]; + let mut writer = MultiWriter::new(&mut writers, 2); + + let err = writer + .write(vec![Bytes::from_static(b"quorum impossible")]) + .await + .expect_err("write quorum above writer count must fail"); + let err = err.to_string(); + assert!(err.contains("Failed to write data")); + assert!(err.contains("required=2")); + assert!(err.contains("erasure write quorum")); + + let shutdown_err = writer + .shutdown() + .await + .expect_err("shutdown quorum above writer count must fail"); + let shutdown_err = shutdown_err.to_string(); + assert!(shutdown_err.contains("Failed to shutdown writers")); + assert!(shutdown_err.contains("required=2")); + assert!(shutdown_err.contains("erasure write quorum")); + } + #[tokio::test] async fn drain_queued_inflight_bytes_consumes_pending_blocks() { let (tx, mut rx) = mpsc::channel(2); @@ -900,6 +964,40 @@ mod tests { assert!(!committed.lock().unwrap().is_empty()); } + #[tokio::test] + #[serial_test::serial] + async fn encode_bytesmut_ingest_streaming_path_writes_and_shutdowns_writers() { + temp_env::async_with_vars([(ENV_RUSTFS_ERASURE_ENCODE_BYTESMUT_INGEST, Some("true"))], async { + const DATA_SHARDS: usize = 2; + const PARITY_SHARDS: usize = 2; + const TOTAL_SHARDS: usize = DATA_SHARDS + PARITY_SHARDS; + const BLOCK_SIZE: usize = 32; + + let committed: Vec>>> = (0..TOTAL_SHARDS).map(|_| Arc::new(Mutex::new(Vec::new()))).collect(); + let mut writers: Vec> = committed + .iter() + .map(|c| Some(bitrot_writer(DeferredCommitWriter::new(c.clone()), BLOCK_SIZE / DATA_SHARDS))) + .collect(); + + let payload = vec![0x5a; BLOCK_SIZE * 2 + 7]; + let erasure = Arc::new(Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE)); + let reader = tokio::io::BufReader::new(Cursor::new(payload.clone())); + let (_reader, written) = erasure + .encode(reader, &mut writers, DATA_SHARDS) + .await + .expect("BytesMut ingest path should encode the streaming payload"); + + assert_eq!(written, payload.len()); + for (index, committed) in committed.iter().enumerate() { + assert!( + !committed.lock().expect("committed buffer should be lockable").is_empty(), + "shard {index} should receive bytesmut-ingest data" + ); + } + }) + .await; + } + #[tokio::test] async fn encode_streaming_write_quorum_failure_aborts_and_reports_error() { const DATA_SHARDS: usize = 2; @@ -1089,6 +1187,28 @@ mod tests { } } + #[tokio::test(flavor = "current_thread")] + async fn encode_block_bytes_mut_works_on_current_thread_runtime() { + let erasure = Arc::new(Erasure::new(2, 2, 64)); + let payload = b"bytesmut current-thread payload"; + let shards = erasure + .clone() + .encode_block_bytes_mut(bytes::BytesMut::from(&payload[..]), payload.len()) + .await + .expect("bytesmut encode should succeed on current-thread runtime"); + + let expected_shard_size = payload.len().div_ceil(erasure.data_shards); + assert_eq!(shards.len(), erasure.total_shard_count()); + assert!(shards.iter().all(|shard| shard.len() == expected_shard_size)); + + let mut restored = Vec::new(); + for shard in shards.iter().take(erasure.data_shards) { + restored.extend_from_slice(shard); + } + restored.truncate(payload.len()); + assert_eq!(restored, payload); + } + #[tokio::test] async fn encode_batched_writes_full_and_tail_batches() { const DATA_SHARDS: usize = 2; diff --git a/crates/ecstore/src/erasure/coding/erasure.rs b/crates/ecstore/src/erasure/coding/erasure.rs index 93e5c9455..a3f334f9a 100644 --- a/crates/ecstore/src/erasure/coding/erasure.rs +++ b/crates/ecstore/src/erasure/coding/erasure.rs @@ -942,6 +942,9 @@ mod tests { use super::*; use proptest::collection::{btree_set, vec}; use proptest::prelude::*; + use std::pin::Pin; + use std::task::{Context, Poll}; + use tokio::io::ReadBuf; fn optional_shards(shards: &[Bytes]) -> Vec>> { shards.iter().map(|shard| Some(shard.to_vec())).collect() @@ -984,6 +987,29 @@ mod tests { assert_eq!(owned, borrowed); } + struct ErrorAfterPartialReader { + emitted: bool, + } + + impl AsyncRead for ErrorAfterPartialReader { + fn poll_read(mut self: Pin<&mut Self>, _cx: &mut Context<'_>, buf: &mut ReadBuf<'_>) -> Poll> { + if !self.emitted { + self.emitted = true; + buf.put_slice(&[1]); + return Poll::Ready(Ok(())); + } + Poll::Ready(Err(io::Error::new(io::ErrorKind::BrokenPipe, "partial read failure"))) + } + } + + struct ImmediateErrorReader; + + impl AsyncRead for ImmediateErrorReader { + fn poll_read(self: Pin<&mut Self>, _cx: &mut Context<'_>, _buf: &mut ReadBuf<'_>) -> Poll> { + Poll::Ready(Err(io::Error::other("immediate read failure"))) + } + } + #[test] fn has_valid_dimensions_rejects_zero_block_size_or_data_shards() { // Well-formed erasure metadata is accepted. @@ -999,6 +1025,130 @@ mod tests { assert!(!Erasure::new(0, 0, 0).has_valid_dimensions()); } + #[tokio::test] + async fn encode_stream_callback_async_stops_on_reader_errors() { + let erasure = std::sync::Arc::new(Erasure::new(2, 1, 4)); + let mut partial_error = ErrorAfterPartialReader { emitted: false }; + let mut partial_callbacks = Vec::new(); + let total = erasure + .clone() + .encode_stream_callback_async(&mut partial_error, |result| { + partial_callbacks.push(result.map(|blocks| blocks.len()).map_err(|err| err.kind())); + async { Ok::<(), io::Error>(()) } + }) + .await + .expect("partial read error should not make callback fail"); + assert_eq!(total, 0); + assert!( + partial_callbacks.is_empty(), + "unexpected EOF after a partial read should stop without emitting a block" + ); + + let mut immediate_error = ImmediateErrorReader; + let mut immediate_callbacks = Vec::new(); + let total = erasure + .encode_stream_callback_async(&mut immediate_error, |result| { + immediate_callbacks.push(result.map(|blocks| blocks.len()).map_err(|err| err.kind())); + async { Ok::<(), io::Error>(()) } + }) + .await + .expect("immediate read error should be delivered to callback"); + assert_eq!(total, 0); + assert_eq!(immediate_callbacks, vec![Err(io::ErrorKind::Other)]); + } + + #[test] + fn default_and_legacy_clone_preserve_safe_zero_state_and_restore_data() { + let default = Erasure::default(); + assert_eq!(default.total_shard_count(), 0); + assert!(!default.has_valid_dimensions()); + assert_eq!(default.shard_file_size(0), 0); + assert_eq!(default.shard_file_size(-7), -7); + + let legacy = Erasure::new_with_options(2, 2, 64, true); + let cloned = legacy.clone(); + assert_eq!(cloned.data_shards, legacy.data_shards); + assert_eq!(cloned.parity_shards, legacy.parity_shards); + assert_eq!(cloned.block_size, legacy.block_size); + assert!(cloned.uses_legacy); + + let data = b"legacy clone should keep independent SIMD caches"; + let encoded = cloned.encode_data(data).expect("legacy clone should encode"); + let mut shards = optional_shards(&encoded); + shards[0] = None; + cloned.decode_data(&mut shards).expect("legacy clone should decode"); + assert_eq!(recover_data(&shards, cloned.data_shards, data.len()), data); + } + + #[test] + fn legacy_verify_reports_invalid_empty_valid_and_corrupt_parity_sets() { + let legacy = LegacyReedSolomonEncoder::new(2, 2).expect("legacy encoder should construct"); + let wrong_count: [&[u8]; 1] = [&[]]; + let err = legacy.verify(&wrong_count).expect_err("wrong shard count must be rejected"); + assert!(err.to_string().contains("invalid shard count")); + + let empty: [&[u8]; 4] = [&[], &[], &[], &[]]; + assert!( + legacy + .verify(&empty) + .expect("all-empty legacy shards are internally consistent") + ); + + let erasure = Erasure::new_with_options(2, 2, 64, true); + let encoded = erasure + .encode_data(b"legacy verify should compare regenerated parity") + .expect("legacy encode should succeed"); + let refs = encoded.iter().map(Bytes::as_ref).collect::>(); + assert!(legacy.verify(&refs).expect("valid legacy shards should verify")); + + let mut corrupt = encoded.iter().map(|shard| shard.to_vec()).collect::>(); + corrupt[erasure.data_shards][0] ^= 0x80; + let corrupt_refs = corrupt.iter().map(Vec::as_slice).collect::>(); + assert!(!legacy.verify(&corrupt_refs).expect("corrupt parity should be detected")); + } + + #[test] + fn parity_helper_and_empty_payload_recovery_fail_closed_on_malformed_shards() { + let mut wrong_count = vec![Some(vec![1])]; + let err = encode_parity_shards(&mut wrong_count, 2, 1, |_| panic!("wrong count must fail before encode")) + .expect_err("wrong shard count must be rejected"); + assert!(err.to_string().contains("invalid shard count")); + + let mut inconsistent_empty = vec![Some(Vec::new()), Some(vec![1])]; + let err = encode_parity_shards(&mut inconsistent_empty, 1, 1, |_| panic!("zero-length mismatch must fail before encode")) + .expect_err("mixed empty and non-empty shards must be rejected"); + assert!(err.to_string().contains("inconsistent shard length")); + + let mut inconsistent_non_empty = vec![Some(vec![1]), Some(vec![2, 3])]; + let err = encode_parity_shards(&mut inconsistent_non_empty, 1, 1, |_| { + panic!("non-empty length mismatch must fail before encode") + }) + .expect_err("mismatched non-empty shards must be rejected"); + assert!(err.to_string().contains("inconsistent shard length")); + + let mut no_present_empty_payload = vec![None, None, None]; + assert!( + !recover_empty_payload_data_shards(&mut no_present_empty_payload, 2, 1) + .expect("all-missing empty payload marker should not be synthesized") + ); + } + + #[test] + fn verify_data_and_parity_handles_zero_parity_and_rejects_wrong_count() { + let no_parity = Erasure::new(2, 0, 64); + assert!( + no_parity + .verify_data_and_parity(&[Some(vec![1]), Some(vec![2, 3])]) + .expect("zero parity requires no parity verification") + ); + + let erasure = Erasure::new(2, 2, 64); + let err = erasure + .verify_data_and_parity(&[Some(Vec::new())]) + .expect_err("wrong shard count must fail verification"); + assert!(err.to_string().contains("invalid shard count")); + } + #[test] fn encode_data_owned_matches_borrowed_path() { for uses_legacy in [false, true] { @@ -1753,37 +1903,22 @@ mod tests { // Create data that will result in 64+ byte shards let data = vec![0x42u8; 200]; // 200 bytes, should create ~50 byte shards per data shard - let result = erasure.encode_data(&data); + let shards = erasure.encode_data(&data).expect("minimum shard size data should encode"); + println!("SIMD encoding succeeded with shard size: {}", shards[0].len()); - // This might fail due to SIMD shard size requirements - match result { - Ok(shards) => { - println!("SIMD encoding succeeded with shard size: {}", shards[0].len()); + // Test decoding + let mut shards_opt: Vec>> = shards.iter().map(|b| Some(b.to_vec())).collect(); + shards_opt[1] = None; - // Test decoding - let mut shards_opt: Vec>> = shards.iter().map(|b| Some(b.to_vec())).collect(); - shards_opt[1] = None; - - let decode_result = erasure.decode_data(&mut shards_opt); - match decode_result { - Ok(_) => { - let mut recovered = Vec::new(); - for shard in shards_opt.iter().take(data_shards) { - recovered.extend_from_slice(shard.as_ref().expect("operation should succeed")); - } - recovered.truncate(data.len()); - assert_eq!(&recovered, &data); - } - Err(e) => { - println!("SIMD decoding failed with shard size {}: {}", shards[0].len(), e); - } - } - } - Err(e) => { - println!("SIMD encoding failed with small shard size: {e}"); - // This is expected for very small shard sizes - } + erasure + .decode_data(&mut shards_opt) + .expect("minimum shard size data should decode"); + let mut recovered = Vec::new(); + for shard in shards_opt.iter().take(data_shards) { + recovered.extend_from_slice(shard.as_ref().expect("operation should succeed")); } + recovered.truncate(data.len()); + assert_eq!(&recovered, &data); } #[test] @@ -1877,46 +2012,26 @@ mod tests { let small_data = b"tiny!123".to_vec(); // 8 bytes data // Test encoding with small data - let result = erasure.encode_data(&small_data); - match result { - Ok(shards) => { - println!("✅ SIMD encoding succeeded: {} bytes into {} shards", small_data.len(), shards.len()); - assert_eq!(shards.len(), data_shards + parity_shards); + let shards = erasure.encode_data(&small_data).expect("small data should encode"); + println!("SIMD encoding succeeded: {} bytes into {} shards", small_data.len(), shards.len()); + assert_eq!(shards.len(), data_shards + parity_shards); - // Test decoding - let mut shards_opt: Vec>> = shards.iter().map(|shard| Some(shard.to_vec())).collect(); + // Test decoding + let mut shards_opt: Vec>> = shards.iter().map(|shard| Some(shard.to_vec())).collect(); - // Lose some shards to test recovery - shards_opt[1] = None; // Lose one data shard - shards_opt[4] = None; // Lose one parity shard + // Lose some shards to test recovery + shards_opt[1] = None; // Lose one data shard + shards_opt[4] = None; // Lose one parity shard - let decode_result = erasure.decode_data(&mut shards_opt); - match decode_result { - Ok(()) => { - println!("✅ SIMD decode worked"); - - // Verify recovered data - let mut recovered = Vec::new(); - for shard in shards_opt.iter().take(data_shards) { - recovered.extend_from_slice(shard.as_ref().expect("operation should succeed")); - } - recovered.truncate(small_data.len()); - println!("recovered: {recovered:?}"); - println!("small_data: {small_data:?}"); - assert_eq!(&recovered, &small_data); - println!("✅ Data recovery successful with SIMD"); - } - Err(e) => { - println!("❌ SIMD decode failed: {e}"); - // For very small data, decode failure might be acceptable - } - } - } - Err(e) => { - println!("❌ SIMD encode failed: {e}"); - // For very small data or configuration issues, encoding might fail - } + erasure.decode_data(&mut shards_opt).expect("small data should decode"); + let mut recovered = Vec::new(); + for shard in shards_opt.iter().take(data_shards) { + recovered.extend_from_slice(shard.as_ref().expect("operation should succeed")); } + recovered.truncate(small_data.len()); + println!("recovered: {recovered:?}"); + println!("small_data: {small_data:?}"); + assert_eq!(&recovered, &small_data); } #[test] diff --git a/crates/ecstore/src/erasure/coding/heal.rs b/crates/ecstore/src/erasure/coding/heal.rs index 7ab16eb02..d4d18401a 100644 --- a/crates/ecstore/src/erasure/coding/heal.rs +++ b/crates/ecstore/src/erasure/coding/heal.rs @@ -208,10 +208,10 @@ mod tests { use super::*; use crate::erasure::coding::{CustomWriter, Erasure}; use rustfs_utils::HashAlgorithm; - use std::io::Cursor; + use std::io::{self, Cursor}; use std::pin::Pin; use std::task::{Context, Poll}; - use tokio::io::AsyncWrite; + use tokio::io::{AsyncWrite, ReadBuf}; /// An `AsyncWrite` that accepts writes until `fail_at` bytes have been /// written, then fails every subsequent write. Used to simulate a @@ -239,6 +239,91 @@ mod tests { } } + struct PendingReader; + + impl AsyncRead for PendingReader { + fn poll_read(self: Pin<&mut Self>, _cx: &mut Context<'_>, _buf: &mut ReadBuf<'_>) -> Poll> { + Poll::Pending + } + } + + struct FailingReader; + + impl AsyncRead for FailingReader { + fn poll_read(self: Pin<&mut Self>, _cx: &mut Context<'_>, _buf: &mut ReadBuf<'_>) -> Poll> { + Poll::Ready(Err(io::Error::other("synthetic heal read failure"))) + } + } + + fn inline_writer(shard_size: usize) -> BitrotWriterWrapper { + BitrotWriterWrapper::new(CustomWriter::new_inline_buffer(), shard_size, HashAlgorithm::None) + } + + #[tokio::test] + async fn read_heal_shards_returns_empty_slots_for_zero_sized_shards() { + let mut readers = vec![ + Some(BitrotReader::new(Cursor::new(vec![1, 2, 3]), 0, HashAlgorithm::None, false)), + None, + ]; + + let (shards, errs) = read_heal_shards(&mut readers, 0, Duration::ZERO).await; + + assert_eq!(shards, vec![None, None]); + assert_eq!(errs, vec![None, None]); + } + + #[tokio::test] + async fn read_heal_shards_records_reader_errors_and_retiring_timeouts() { + let mut readers = vec![Some(BitrotReader::new(FailingReader, 4, HashAlgorithm::None, false))]; + + let (shards, errs) = read_heal_shards(&mut readers, 4, Duration::ZERO).await; + + assert!(shards[0].is_none()); + assert!( + errs[0] + .as_ref() + .is_some_and(|err| err.to_string().contains("synthetic heal read failure")) + ); + assert!(readers[0].is_some(), "ordinary read errors must not retire readers"); + + let mut timeout_readers = vec![Some(BitrotReader::new(PendingReader, 4, HashAlgorithm::None, false))]; + let (shards, errs) = read_heal_shards(&mut timeout_readers, 4, Duration::from_millis(1)).await; + + assert_eq!(shards, vec![None]); + assert!(errs[0].as_ref().is_some_and(|err| err.to_string().contains("timed out"))); + assert!(timeout_readers[0].is_none(), "timed-out readers are retired"); + } + + #[tokio::test] + async fn heal_rejects_invalid_writer_shape_before_reading() { + let erasure = Erasure::new(2, 1, 64); + let mut writers = vec![Some(inline_writer(erasure.shard_size()))]; + let readers: Vec>>>> = Vec::new(); + + let err = erasure + .heal(&mut writers, readers, 0, &[]) + .await + .expect_err("invalid writer count must fail"); + + assert!(err.to_string().contains("invalid argument")); + } + + #[tokio::test] + async fn heal_empty_object_only_shuts_down_available_writers() { + let erasure = Erasure::new(2, 1, 64); + let mut writers = (0..erasure.total_shard_count()) + .map(|_| Some(inline_writer(erasure.shard_size()))) + .collect::>(); + let readers: Vec>>>> = Vec::new(); + + erasure + .heal(&mut writers, readers, 0, &[]) + .await + .expect("empty object heal should only close writers"); + + assert!(writers.iter().all(Option::is_some)); + } + #[tokio::test] async fn heal_reconstructs_missing_parity_shard() { let erasure = Erasure::new(2, 2, 64); @@ -266,11 +351,7 @@ mod tests { let mut writers = (0..erasure.total_shard_count()) .map(|index| { if index == missing_parity { - Some(BitrotWriterWrapper::new( - CustomWriter::new_inline_buffer(), - erasure.shard_size(), - HashAlgorithm::None, - )) + Some(inline_writer(erasure.shard_size())) } else { None } @@ -319,11 +400,7 @@ mod tests { let mut writers = (0..erasure.total_shard_count()) .map(|index| { if index == missing_data { - Some(BitrotWriterWrapper::new( - CustomWriter::new_inline_buffer(), - erasure.shard_size(), - HashAlgorithm::None, - )) + Some(inline_writer(erasure.shard_size())) } else { None } @@ -403,11 +480,7 @@ mod tests { let mut writers = (0..erasure.total_shard_count()) .map(|index| { if index == missing_data { - Some(BitrotWriterWrapper::new( - CustomWriter::new_inline_buffer(), - erasure.shard_size(), - HashAlgorithm::None, - )) + Some(inline_writer(erasure.shard_size())) } else { None } diff --git a/crates/ecstore/src/lib.rs b/crates/ecstore/src/lib.rs index 7ab00b64a..e79fc6d15 100644 --- a/crates/ecstore/src/lib.rs +++ b/crates/ecstore/src/lib.rs @@ -85,3 +85,6 @@ mod rio_tests { assert_eq!(crate::io_support::rio::backend_name(), expected); } } + +#[cfg(test)] +mod ecstore_validation_blackbox; diff --git a/crates/ecstore/src/set_disk/core/io_primitives.rs b/crates/ecstore/src/set_disk/core/io_primitives.rs index 33f03b092..0351a3518 100644 --- a/crates/ecstore/src/set_disk/core/io_primitives.rs +++ b/crates/ecstore/src/set_disk/core/io_primitives.rs @@ -3060,35 +3060,28 @@ impl SetDisks { } if let Some(err) = reduce_write_quorum_errs(&errs, OBJECT_OP_IGNORED_ERRS, write_quorum) { - let mut revert_futures = Vec::with_capacity(disks.len()); for (i, err) in errs.iter().enumerate() { if err.is_some() { continue; } if let Some(disk) = disks[i].as_ref() { - let disk = disk.clone(); - let bucket = bucket.to_string(); let path = path_join_buf(&[prefix, STORAGE_FORMAT_FILE]); - revert_futures.push(async move { - if let Err(err) = disk - .delete( - &bucket, - &path, - DeleteOptions { - recursive: true, - ..Default::default() - }, - ) - .await - { - warn!("write meta revert err {:?}", err); - } - }); + if let Err(err) = disk + .delete( + bucket, + &path, + DeleteOptions { + recursive: true, + ..Default::default() + }, + ) + .await + { + warn!("write meta revert err {:?}", err); + } } } - - join_all(revert_futures).await; return Err(err); } Ok(()) @@ -3811,6 +3804,7 @@ pub(in crate::set_disk) mod cleanup_fault_injection { mod tests { use super::*; use std::io::Cursor; + use tempfile::TempDir; use tokio::io::AsyncReadExt; fn metadata_test_fileinfo(object: &str) -> FileInfo { @@ -3840,6 +3834,49 @@ mod tests { } } + async fn read_multiple_test_disk(bucket: &str, objects: &[(&str, &[u8])]) -> (TempDir, DiskStore) { + let dir = tempfile::tempdir().expect("tempdir should be created"); + let endpoint = + Endpoint::try_from(dir.path().to_str().expect("tempdir path should be utf8")).expect("endpoint should parse"); + let disk = new_disk( + &endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("disk should be created"); + + match disk.make_volume(bucket).await { + Ok(()) | Err(DiskError::VolumeExists) => {} + Err(err) => panic!("bucket should be available: {err:?}"), + } + for (object, body) in objects { + disk.write_all(bucket, object, Bytes::copy_from_slice(body)) + .await + .expect("object should be written"); + } + + (dir, disk) + } + + async fn io_primitives_test_set(disks: Vec>, default_parity_count: usize) -> Arc { + let set_drive_count = disks.len(); + SetDisks::new( + "io-primitives-test".to_string(), + Arc::new(RwLock::new(disks)), + set_drive_count, + default_parity_count, + 0, + 0, + Vec::new(), + FormatV3::new(1, 1), + Vec::new(), + ) + .await + } + fn failed_read_repair_submitter(_request: rustfs_common::heal_channel::HealChannelRequest) -> ReadRepairAdmissionFuture { Box::pin(async { ReadRepairAdmissionOutcome::Failed("injected submit failure".to_string()) }) } @@ -3848,6 +3885,38 @@ mod tests { Box::pin(async { ReadRepairAdmissionOutcome::Response(HealAdmissionResult::Accepted) }) } + fn dropped_read_repair_submitter(_request: rustfs_common::heal_channel::HealChannelRequest) -> ReadRepairAdmissionFuture { + Box::pin(async { + ReadRepairAdmissionOutcome::Response(HealAdmissionResult::Dropped( + rustfs_common::heal_channel::HealAdmissionDropReason::PolicyDropped, + )) + }) + } + + fn test_object_bitrot_reader() -> ObjectBitrotReader { + BitrotReader::new( + Box::new(Cursor::new(vec![1u8, 2, 3, 4])) as Box, + 4, + HashAlgorithm::None, + false, + ) + } + + fn test_deferred_object_bitrot_reader() -> (ObjectBitrotReader, DeferredReaderStripeHandle) { + create_deferred_bitrot_reader_with_stripe_handle( + Some(Bytes::from_static(&[1, 2, 3, 4])), + None, + "bucket", + "object/part.1", + 0, + 4, + 4, + HashAlgorithm::None, + false, + false, + ) + } + #[test] fn dangling_delete_grace_defaults_to_one_hour() { temp_env::with_var(ENV_HEAL_DANGLING_DELETE_GRACE_SECS, None::<&str>, || { @@ -4009,6 +4078,36 @@ mod tests { assert!(part.etag.is_empty()); } + #[test] + fn resolve_read_part_returns_part_when_etag_reaches_quorum() { + let responses = vec![ + Some(vec![read_part_test_part(1, "winner")]), + Some(vec![read_part_test_part(1, "loser")]), + Some(vec![read_part_test_part(1, "winner")]), + ]; + + let part = resolve_read_part_from_responses("bucket", "upload/part.1.meta", 1, 0, 1, &responses, 2) + .expect("etag quorum should resolve the present part"); + + assert_eq!(part.etag, "winner"); + } + + #[test] + fn resolve_read_part_diagnostic_branch_keeps_read_quorum_error() { + temp_env::with_var("RUSTFS_ISSUE3031_DIAG_ENABLE", Some("true"), || { + let responses = vec![ + Some(Vec::new()), + None, + Some(vec![read_part_test_error(1, "permission denied")]), + ]; + + let err = resolve_read_part_from_responses("bucket", "upload/part.1.meta", 1, 0, 1, &responses, 2) + .expect_err("diagnostic logging must not change the read-quorum result"); + + assert_eq!(err, DiskError::ErasureReadQuorum); + }); + } + // Runs under a Tokio runtime like the sibling reservation tests: // shard_read_costs_for_disks consults process-global topology state // (local_endpoint_hosts_for_shard_costs), whose fast-lock manager lazily @@ -4021,6 +4120,349 @@ mod tests { assert!(shard_read_costs_for_disks(&[]).is_empty()); } + #[test] + fn shard_read_cost_for_endpoint_and_missing_disk_cover_all_cost_classes() { + let same_node_hosts = vec!["node-a:9000".to_string()]; + + assert_eq!(shard_read_cost_for_disk(None, &same_node_hosts), ShardReadCost::Unknown); + assert_eq!(shard_read_cost_for_endpoint(true, "", &same_node_hosts), ShardReadCost::Local); + assert_eq!( + shard_read_cost_for_endpoint(false, "node-a:9000", &same_node_hosts), + ShardReadCost::SameNode + ); + assert_eq!( + shard_read_cost_for_endpoint(false, "node-b:9000", &same_node_hosts), + ShardReadCost::Remote + ); + assert!(local_endpoint_hosts_for_shard_costs().is_empty()); + } + + #[test] + #[serial_test::serial] + fn bitrot_reader_setup_tracks_strategy_counters_and_deferred_readers() { + temp_env::with_var(ENV_RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP, Some("true"), || { + assert!(matches!( + get_bitrot_reader_setup_strategy(BitrotReaderSetupMode::ReadQuorum, false), + BitrotReaderSetupStrategy::DataBlocksFirst + )); + }); + temp_env::with_var(ENV_RUSTFS_GET_CODEC_STREAMING_DATA_BLOCKS_FIRST_READER_SETUP, Some("true"), || { + assert!(matches!( + get_bitrot_reader_setup_strategy(BitrotReaderSetupMode::VerifyReconstruction, false), + BitrotReaderSetupStrategy::DataBlocksFirst + )); + }); + assert_eq!(BitrotReaderSetupMode::ReadQuorum.as_str(), "read_quorum"); + assert_eq!(BitrotReaderSetupMode::VerifyReconstruction.as_str(), "verify_reconstruction"); + assert_eq!(BitrotReaderSetupStrategy::AllShards.as_str(), "all_shards"); + assert_eq!(BitrotReaderSetupStrategy::DataBlocksFirst.as_str(), "data_blocks_first"); + assert_eq!(BitrotReaderSetupStrategy::DataBlocksOnly.as_str(), "data_blocks_only"); + + let mut setup = BitrotReaderSetup::new(4); + assert_eq!(setup.scheduled_shards(), 0); + assert!(setup.mark_scheduled(0)); + assert!(!setup.mark_scheduled(0)); + assert_eq!(setup.scheduled_shards(), 1); + assert_eq!(setup.pending_scheduled_shards(), 1); + assert_eq!(setup.available_shards(), 0); + assert_eq!(setup.completed_failed_shards(), 0); + assert_eq!(setup.reconstruction_verification_target(3, 2), 3); + assert!(!setup.has_setup_quorum(3, 2, BitrotReaderSetupMode::ReadQuorum)); + assert!(!setup.data_shards_attempted(3)); + assert_eq!(setup.scheduling_target(3, 2, BitrotReaderSetupMode::VerifyReconstruction), 3); + + setup.apply_reader_result(0, Ok(Some(test_object_bitrot_reader()))); + setup.apply_reader_result(1, Ok(None)); + setup.apply_reader_result(2, Err(DiskError::FileCorrupt)); + + assert_eq!(setup.attempted_shards(), 3); + assert_eq!(setup.pending_scheduled_shards(), 0); + assert_eq!(setup.available_shards(), 1); + assert_eq!(setup.available_data_shards(3), 1); + assert_eq!(setup.completed_failed_shards(), 2); + assert!(setup.data_shards_attempted(3)); + assert_eq!(setup.reconstruction_verification_target(3, 2), 3); + assert_eq!(setup.setup_target(3, 2, BitrotReaderSetupMode::VerifyReconstruction), 3); + assert_eq!(setup.scheduling_target(3, 2, BitrotReaderSetupMode::VerifyReconstruction), 3); + + let mut verification_setup = BitrotReaderSetup::new(4); + verification_setup.apply_reader_result(0, Ok(Some(test_object_bitrot_reader()))); + verification_setup.apply_reader_result(1, Ok(Some(test_object_bitrot_reader()))); + verification_setup.apply_reader_result(2, Err(DiskError::FileCorrupt)); + verification_setup.apply_reader_result(3, Ok(Some(test_object_bitrot_reader()))); + assert_eq!(verification_setup.reconstruction_verification_target(3, 2), 4); + assert_eq!(verification_setup.setup_target(3, 2, BitrotReaderSetupMode::VerifyReconstruction), 4); + assert!(verification_setup.has_setup_quorum(3, 2, BitrotReaderSetupMode::ReadQuorum)); + + let (deferred_reader, stripe_handle) = test_deferred_object_bitrot_reader(); + setup.retain_deferred_reader(3, deferred_reader, stripe_handle); + assert_eq!(setup.deferred_shards(), 1); + assert!(setup.readers[3].is_some()); + assert!(setup.errors[3].is_none()); + } + + #[tokio::test] + async fn write_unique_file_info_reverts_metadata_when_write_quorum_fails() { + let bucket = "write-unique-bucket"; + let object = "object"; + let (_dir, disk) = read_multiple_test_disk(bucket, &[]).await; + let files = vec![metadata_test_fileinfo(object), metadata_test_fileinfo(object)]; + + let result = SetDisks::write_unique_file_info(&[Some(disk.clone()), None], bucket, bucket, object, &files, 2).await; + + assert!(result.is_err(), "missing disk must prevent the requested write quorum"); + assert!( + matches!( + disk.read_all(bucket, &path_join_buf(&[object, STORAGE_FORMAT_FILE])).await, + Err(DiskError::FileNotFound) + ), + "successful metadata write must be reverted when quorum is not reached" + ); + } + + #[tokio::test] + async fn update_object_meta_handles_empty_metadata_and_missing_quorum() { + let set = io_primitives_test_set(vec![None, None], 1).await; + let mut empty = metadata_test_fileinfo("object"); + empty.metadata.clear(); + + set.update_object_meta("bucket", "object", empty, &[None, None]) + .await + .expect("empty metadata update without replacement should be a no-op"); + + let mut with_metadata = metadata_test_fileinfo("object"); + with_metadata + .metadata + .insert("x-amz-meta-test".to_string(), "value".to_string()); + let result = set.update_object_meta("bucket", "object", with_metadata, &[None, None]).await; + + assert!(result.is_err(), "missing disks must prevent metadata write quorum"); + } + + #[tokio::test] + async fn load_file_info_versions_exact_returns_versions_from_read_quorum() { + let bucket = "exact-versions-bucket"; + let object = "exact-object"; + let (_dir, disk) = read_multiple_test_disk(bucket, &[]).await; + let mut fi = metadata_test_fileinfo(object); + fi.version_id = Some(Uuid::new_v4()); + fi.mod_time = Some(OffsetDateTime::now_utc()); + disk.write_metadata(bucket, bucket, object, fi.clone()) + .await + .expect("metadata should be written"); + let set = io_primitives_test_set(vec![Some(disk)], 0).await; + + let versions = set + .load_file_info_versions_exact(bucket, object) + .await + .expect("exact version load should succeed") + .expect("exact version load should find metadata"); + + assert_eq!(versions.versions.len(), 1); + assert_eq!(versions.versions[0].version_id, fi.version_id); + assert_eq!(versions.versions[0].name, object); + } + + #[tokio::test] + async fn commit_rename_data_dir_reclaims_old_data_dir_and_reports_receipt() { + let bucket = "commit-rename-bucket"; + let object = "object"; + let old_data_dir = "11111111-1111-1111-1111-111111111111"; + let committed_data_dir = "22222222-2222-2222-2222-222222222222"; + let path = format!("{object}/{old_data_dir}/part.1"); + let (_dir1, disk1) = read_multiple_test_disk(bucket, &[(&path, b"one".as_slice())]).await; + let (_dir2, disk2) = read_multiple_test_disk(bucket, &[(&path, b"two".as_slice())]).await; + let set = io_primitives_test_set(vec![Some(disk1.clone()), Some(disk2.clone())], 1).await; + + let cleanup = set + .commit_rename_data_dir( + &[Some(disk1.clone()), Some(disk2.clone())], + bucket, + object, + old_data_dir, + committed_data_dir, + 2, + ) + .await; + assert_eq!(cleanup.attempted, 2); + assert_eq!(cleanup.reclaimed, 2); + assert!(!cleanup.has_residue()); + + assert!(matches!(disk1.read_all(bucket, &path).await, Err(DiskError::FileNotFound))); + assert!(matches!(disk2.read_all(bucket, &path).await, Err(DiskError::FileNotFound))); + + let missing = set + .commit_rename_data_dir(&[None, None], bucket, object, old_data_dir, committed_data_dir, 1) + .await; + assert_eq!(missing.attempted, 0); + assert_eq!(missing.reclaimed, 0); + assert!(!missing.has_residue(), "missing disk slots must not be counted as cleanup residue"); + assert!(missing.below_quorum, "missing disk slots should remain visible in the quorum lens"); + } + + #[tokio::test] + async fn delete_prefix_removes_present_disks_and_ignores_missing_disk_slots() { + let bucket = "delete-prefix-bucket"; + let (_dir, disk) = read_multiple_test_disk(bucket, &[("prefix/object.txt", b"payload".as_slice())]).await; + let set = io_primitives_test_set(vec![Some(disk.clone()), None], 1).await; + + set.delete_prefix(bucket, "prefix") + .await + .expect("missing disk slots should not block prefix deletion"); + + assert!(matches!(disk.read_all(bucket, "prefix/object.txt").await, Err(DiskError::FileNotFound))); + } + + #[tokio::test] + async fn delete_if_dangling_respects_recent_write_grace_without_deleting_metadata() { + let bucket = "dangling-grace-bucket"; + let object = "object"; + let (_dir, disk) = read_multiple_test_disk(bucket, &[]).await; + let set = io_primitives_test_set(vec![Some(disk.clone()), None, None], 1).await; + let mut fi = metadata_test_fileinfo(object); + fi.mod_time = Some(OffsetDateTime::now_utc()); + disk.write_metadata(bucket, bucket, object, fi.clone()) + .await + .expect("metadata should be written before dangling check"); + + let err = set + .delete_if_dangling( + bucket, + object, + &[fi, FileInfo::default(), FileInfo::default()], + &[None, Some(DiskError::FileNotFound), Some(DiskError::FileNotFound)], + &HashMap::new(), + ObjectOptions::default(), + ) + .await + .expect_err("recent dangling metadata must stay protected by grace"); + + assert_eq!(err, DiskError::ErasureReadQuorum); + disk.read_all(bucket, &path_join_buf(&[object, STORAGE_FORMAT_FILE])) + .await + .expect("metadata should remain during dangling grace"); + } + + #[tokio::test] + async fn delete_if_dangling_returns_stale_metadata_when_all_slots_are_already_absent() { + let bucket = "dangling-delete-bucket"; + let object = "object"; + let set = io_primitives_test_set(vec![None, None, None], 1).await; + let mut fi = metadata_test_fileinfo(object); + fi.mod_time = Some(OffsetDateTime::now_utc() - time::Duration::hours(2)); + + let deleted = set + .delete_if_dangling( + bucket, + object, + &[fi.clone(), FileInfo::default(), FileInfo::default()], + &[ + Some(DiskError::FileNotFound), + Some(DiskError::FileNotFound), + Some(DiskError::FileNotFound), + ], + &HashMap::new(), + ObjectOptions::default(), + ) + .await + .expect("stale dangling metadata should pass when every delete target was already absent"); + + assert_eq!(deleted.name, object); + assert_eq!(deleted.mod_time, fi.mod_time); + } + + #[tokio::test] + async fn delete_if_dangling_cleans_when_only_part_results_prove_invalid_metadata_is_dangling() { + let bucket = "dangling-invalid-meta-bucket"; + let object = "object"; + let set = io_primitives_test_set(vec![None, None, None, None], 1).await; + let mut data_errs_by_part = HashMap::new(); + data_errs_by_part.insert( + 1, + vec![ + CHECK_PART_FILE_NOT_FOUND, + CHECK_PART_FILE_NOT_FOUND, + CHECK_PART_FILE_NOT_FOUND, + CHECK_PART_DISK_NOT_FOUND, + ], + ); + + let deleted = set + .delete_if_dangling( + bucket, + object, + &[ + FileInfo::default(), + FileInfo::default(), + FileInfo::default(), + FileInfo::default(), + ], + &[ + Some(DiskError::FileNotFound), + Some(DiskError::FileNotFound), + Some(DiskError::FileNotFound), + Some(DiskError::DiskNotFound), + ], + &data_errs_by_part, + ObjectOptions::default(), + ) + .await + .expect("invalid metadata should be cleanable when part results prove the object is dangling"); + + assert!(!deleted.is_valid()); + } + + #[tokio::test] + async fn read_multiple_files_returns_quorum_data_and_fails_closed_for_partial_file() { + let bucket = "read-multiple-bucket"; + let prefix = "prefix"; + let (_dir1, disk1) = read_multiple_test_disk( + bucket, + &[ + ("prefix/shared.txt", b"longer shared payload".as_slice()), + ("prefix/partial.txt", b"only one disk".as_slice()), + ], + ) + .await; + let (_dir2, disk2) = read_multiple_test_disk(bucket, &[("prefix/shared.txt", b"short".as_slice())]).await; + let req = ReadMultipleReq { + bucket: bucket.to_string(), + prefix: prefix.to_string(), + files: vec!["shared.txt".to_string(), "partial.txt".to_string()], + max_size: 0, + metadata_only: false, + abort404: false, + max_results: 0, + }; + + let responses = SetDisks::read_multiple_files(&[Some(disk1), Some(disk2)], req, 2).await; + + assert_eq!(responses.len(), 2); + assert!(responses[0].exists); + assert_eq!(responses[0].data, b"longer shared payload"); + assert!(!responses[1].exists); + assert_eq!(responses[1].error, Error::ErasureReadQuorum.to_string()); + } + + #[tokio::test] + async fn read_multiple_files_returns_read_quorum_error_when_no_disk_can_answer() { + let req = ReadMultipleReq { + bucket: "bucket".to_string(), + prefix: "prefix".to_string(), + files: vec!["missing.txt".to_string()], + max_size: 0, + metadata_only: false, + abort404: false, + max_results: 0, + }; + + let responses = SetDisks::read_multiple_files(&[None, None], req, 1).await; + + assert_eq!(responses.len(), 1); + assert!(!responses[0].exists); + assert_eq!(responses[0].error, Error::ErasureReadQuorum.to_string()); + } + #[tokio::test] async fn reserve_read_repair_heal_dedupes_by_object_version_and_set() { let object = format!("object-{}", Uuid::new_v4()); @@ -4042,6 +4484,63 @@ mod tests { release_read_repair_heal_reservation(&retry_key).await; } + #[test] + fn record_read_repair_dedup_accepts_known_reasons() { + record_read_repair_dedup("duplicate"); + record_read_repair_dedup("policy_drop"); + } + + #[tokio::test] + async fn reserve_read_repair_heal_prunes_oldest_entry_at_capacity() { + let bucket = format!("bucket-{}", Uuid::new_v4()); + let first_object = format!("object-{}", Uuid::new_v4()); + let first_key = reserve_read_repair_heal(&bucket, &first_object, None, 1, 1) + .await + .expect("first reservation should be accepted"); + + let mut keys = vec![first_key.clone()]; + for index in 0..(READ_REPAIR_HEAL_DEDUP_MAX_ENTRIES + 8) { + let object = format!("object-{index}-{}", Uuid::new_v4()); + if let Some(key) = reserve_read_repair_heal(&bucket, &object, None, 1, 1).await { + keys.push(key); + } + } + + let replaced_first = reserve_read_repair_heal(&bucket, &first_object, None, 1, 1).await; + for key in keys { + release_read_repair_heal_reservation(&key).await; + } + if let Some(key) = replaced_first.as_ref() { + release_read_repair_heal_reservation(key).await; + } + + assert!( + replaced_first.is_some(), + "capacity pruning should evict the oldest read-repair reservation" + ); + } + + #[tokio::test] + async fn submit_read_repair_heal_wrapper_records_reservation_without_external_channel() { + let object = format!("object-{}", Uuid::new_v4()); + submit_read_repair_heal("bucket", &object, None, 4, 5, Some(7), "test").await; + + for _ in 0..20 { + if let Some(key) = reserve_read_repair_heal("bucket", &object, None, 4, 5).await { + release_read_repair_heal_reservation(&key).await; + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + + if let Some(key) = reserve_read_repair_heal("bucket", &object, None, 4, 5).await { + release_read_repair_heal_reservation(&key).await; + } else { + let admitted_key = ReadRepairHealCacheKey::new("bucket", &object, None, 4, 5); + release_read_repair_heal_reservation(&admitted_key).await; + } + } + #[tokio::test] async fn submit_read_repair_heal_releases_reservation_after_submitter_failure() { let object = format!("object-{}", Uuid::new_v4()); @@ -4070,6 +4569,34 @@ mod tests { panic!("failed read-repair submission should release its dedup reservation"); } + #[tokio::test] + async fn submit_read_repair_heal_releases_reservation_after_not_admitted_response() { + let object = format!("object-{}", Uuid::new_v4()); + submit_read_repair_heal_with_submitter( + ReadRepairHealSubmission { + bucket: "bucket", + object: &object, + version_id: Some("version-1"), + pool_index: 2, + set_index: 3, + part_number: Some(2), + reason: "test", + }, + dropped_read_repair_submitter, + ) + .await; + + for _ in 0..20 { + if let Some(key) = reserve_read_repair_heal("bucket", &object, Some("version-1"), 2, 3).await { + release_read_repair_heal_reservation(&key).await; + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + + panic!("not-admitted read-repair submission should release its dedup reservation"); + } + #[tokio::test] async fn submit_read_repair_heal_keeps_admitted_reservation_deduped() { let object = format!("object-{}", Uuid::new_v4()); diff --git a/crates/ecstore/src/set_disk/metadata.rs b/crates/ecstore/src/set_disk/metadata.rs index f465e0019..060669956 100644 --- a/crates/ecstore/src/set_disk/metadata.rs +++ b/crates/ecstore/src/set_disk/metadata.rs @@ -1086,6 +1086,85 @@ mod tests { assert_eq!(err, DiskError::ErasureReadQuorum); } + #[test] + fn metadata_quorum_covers_etag_fallback_and_object_quorum_failures() { + let mut parts_metadata = (1..=3) + .map(|index| { + let mut fi = metadata_quorum_test_fileinfo(OffsetDateTime::now_utc(), index); + fi.mod_time = None; + fi + }) + .collect::>(); + parts_metadata[2] + .metadata + .insert("etag".to_string(), "minority-etag".to_string()); + let errs = vec![None; parts_metadata.len()]; + let disks = vec![None; parts_metadata.len()]; + + let (_online, mod_time, etag) = SetDisks::list_online_disks(&disks, &parts_metadata, &errs, 2); + assert!(mod_time.is_none()); + assert_eq!(etag.as_deref(), Some("object-etag")); + + let zero_parity = SetDisks::object_quorum_from_meta(&parts_metadata, &errs, 0) + .expect("zero default parity should require all metadata shards"); + assert_eq!(zero_parity, (3, 3)); + + let invalid = vec![FileInfo::default(); 4]; + let err = SetDisks::object_quorum_from_meta(&invalid, &vec![None; 4], 2) + .expect_err("invalid metadata without a common parity must fail closed"); + assert_eq!(err, DiskError::ErasureReadQuorum); + } + + #[test] + fn fileinfo_quorum_hash_includes_optional_checksums_and_ignores_replication_noise() { + let mod_time = OffsetDateTime::now_utc(); + let mut left = metadata_quorum_test_fileinfo(mod_time, 1); + left.mode = Some(0o640); + left.written_by_version = Some(42); + left.checksum = Some(Bytes::from_static(b"object-checksum")); + left.parts[0].index = Some(Bytes::from_static(b"part-index")); + left.parts[0].error = Some("repair-pending".to_string()); + left.parts[0].checksums = Some(HashMap::from([ + ("sha256".to_string(), "left".to_string()), + ("crc32".to_string(), "right".to_string()), + ])); + left.metadata.insert( + format!("{}{}", http::RUSTFS_INTERNAL_PREFIX, http::SUFFIX_REPLICATION_STATUS), + "replica-a".to_string(), + ); + + let mut right = left.clone(); + right.parts[0].checksums = Some(HashMap::from([ + ("crc32".to_string(), "right".to_string()), + ("sha256".to_string(), "left".to_string()), + ])); + right.metadata.insert( + format!("{}{}", http::MINIO_INTERNAL_PREFIX, http::SUFFIX_REPLICATION_STATUS), + "replica-b".to_string(), + ); + assert_eq!( + SetDisks::file_info_quorum_hash(&left), + SetDisks::file_info_quorum_hash(&right), + "checksum map ordering and replication metadata must not split quorum identity" + ); + + right.parts[0] + .checksums + .as_mut() + .expect("checksums should exist") + .insert("sha256".to_string(), "changed".to_string()); + assert_ne!(SetDisks::file_info_quorum_hash(&left), SetDisks::file_info_quorum_hash(&right)); + } + + #[test] + fn quorum_helpers_reject_zero_quorum_and_shuffle_check_parts_by_distribution() { + let err = SetDisks::find_file_info_in_quorum(&[], &None, &None, 0).expect_err("zero quorum cannot select metadata"); + assert_eq!(err, DiskError::ErasureReadQuorum); + + assert_eq!(SetDisks::shuffle_check_parts(&[2, 1, 0], &[]), vec![2, 1, 0]); + assert_eq!(SetDisks::shuffle_check_parts(&[2, 1, 0], &[3, 1, 2]), vec![1, 0, 2]); + } + #[test] fn metadata_quorum_uses_simple_majority_for_transitioned_objects() { let parts_metadata = (1..=6).map(transition_metadata_quorum_fileinfo).collect::>(); @@ -1274,4 +1353,23 @@ mod tests { let (d3, _) = SetDisks::shuffle_disks_and_parts_metadata_by_index_owned(disks, parts, &fi); assert_eq!(d3.len(), slots); } + + #[tokio::test] + async fn shuffle_variants_skip_missing_disks_and_invalid_metadata() { + let tempdir = tempfile::tempdir().expect("tempdir should be created"); + let (fi, mut parts) = shuffle_fixture(true); + let mut disks = shuffle_test_disks(&tempdir, parts.len()).await; + disks[0] = None; + parts[1] = FileInfo::default(); + + let (by_index_disks, by_index_parts) = SetDisks::shuffle_disks_and_parts_metadata_by_index(&disks, &parts, &fi); + assert!( + by_index_disks.iter().filter(|disk| disk.is_some()).count() <= disks.iter().filter(|disk| disk.is_some()).count() + ); + assert!(by_index_parts.iter().any(|part| !part.is_valid())); + + let (fallback_disks, fallback_parts) = SetDisks::shuffle_disks_and_parts_metadata(&disks, &parts, &fi); + assert!(fallback_disks.iter().any(Option::is_none)); + assert!(fallback_parts.iter().any(|part| !part.is_valid())); + } } diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index 9306c8dd3..fe68f6bcf 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -91,7 +91,7 @@ use crate::storage_api_contracts::{ CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartOperations as _, MultipartUploadResult, PartInfo, }, namespace::NamespaceLocking as _, - object::{DeletedObject, ObjectIO as _, ObjectOperations as _, ObjectToDelete}, + object::{DeletedObject, HTTPPreconditions, ObjectIO as _, ObjectOperations as _, ObjectToDelete}, range::HTTPRangeSpec, }; use crate::store::utils::is_reserved_or_invalid_bucket; @@ -3562,10 +3562,11 @@ mod tests { use crate::disk::health_state::RuntimeDriveHealthState; use crate::disk::new_disk; use crate::layout::endpoints::SetupType; + use crate::object_api::BLOCK_SIZE_V2; use crate::object_api::ObjectInfo; use crate::storage_api_contracts::{ heal::HealOperations as _, lifecycle::TransitionedObject, list::ListOperations as _, multipart::CompletePart, - namespace::NamespaceLocking as _, object::ObjectOperations as _, + namespace::NamespaceLocking as _, object::ObjectIO as _, object::ObjectOperations as _, }; use crate::store::init_format::save_format_file; use crate::store::list_objects::ListPathOptions; @@ -3579,6 +3580,7 @@ mod tests { use tempfile::TempDir; use time::OffsetDateTime; use tokio::fs; + use tokio::io::AsyncReadExt; #[test] fn complete_part_error_maps_confirmed_missing_to_invalid_part() { @@ -7706,6 +7708,757 @@ mod tests { assert!(walked_names.iter().any(|name| name == "object")); } + #[tokio::test] + async fn set_level_put_get_delete_restores_large_object_and_fails_closed_after_delete() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-object-roundtrip"; + let object = "nested/object.bin"; + let payload = (0..(BLOCK_SIZE_V2 + 17)).map(|idx| (idx % 251) as u8).collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + let written = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("large object should be written"); + + assert_eq!(written.size, payload.len() as i64); + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("large object reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("large object should stream"); + assert_eq!(restored, payload); + + let deleted = set_disks + .delete_object(bucket, object, opts.clone()) + .await + .expect("object delete should succeed"); + assert_eq!(deleted.name, object); + + let err = match set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("deleted object must not be readable"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&err), + "deleted object read must fail closed with object-not-found, got {err:?}" + ); + } + + #[tokio::test] + async fn set_level_batched_large_put_get_restores_body() { + const BATCHED_LARGE_SIZE: usize = 64 * 1024 * 1024; + + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-batched-large-object"; + let object = "large/batched.bin"; + let payload = (0..BATCHED_LARGE_SIZE) + .map(|idx| ((idx as u64).wrapping_mul(31).wrapping_add(17) % 251) as u8) + .collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + let written = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("batched-large object should be written"); + assert_eq!(written.size, payload.len() as i64); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("batched-large object reader should open"); + let mut restored = Vec::with_capacity(payload.len()); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("batched-large object should stream"); + assert_eq!(restored, payload); + } + + #[tokio::test] + async fn set_level_put_object_fails_closed_when_writer_quorum_is_unavailable() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-put-writer-quorum"; + let object = "object.txt"; + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created before disk loss"); + { + let mut disks = set_disks.disks.write().await; + disks[1] = None; + } + + let mut reader = PutObjReader::from_vec(b"quorum guarded body".to_vec()); + let err = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect_err("missing writer quorum must fail the put"); + assert!( + matches!(err, Error::ErasureWriteQuorum | Error::InsufficientWriteQuorum(_, _)), + "expected write quorum failure, got {err:?}" + ); + + let read_err = match set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("failed put must not leave a readable object"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&read_err) || matches!(read_err, Error::ErasureReadQuorum), + "failed put must fail closed on read, got {read_err:?}" + ); + } + + #[tokio::test] + async fn set_level_put_object_short_reader_fails_closed_across_write_paths() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-put-short-reader"; + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + + let cases = [ + ("single-block", b"short body".to_vec(), 32), + ("pipeline", b"pipeline short body".to_vec(), BLOCK_SIZE_V2 as i64 + 1), + ("batched-large", b"batched short body".to_vec(), 64 * 1024 * 1024), + ]; + + for (name, payload, declared_size) in cases { + let object = format!("{name}/object.txt"); + let hash_reader = HashReader::from_stream(Cursor::new(payload), declared_size, declared_size, None, None, false) + .expect("test reader should be constructed"); + let mut reader = PutObjReader::new(hash_reader); + let err = set_disks + .put_object(bucket, &object, &mut reader, &opts) + .await + .expect_err("short reader must fail the put"); + let err_text = format!("{err:?}"); + assert!( + err_text.contains("UnexpectedEof") || err_text.contains("IncompleteBody"), + "{name} short reader should fail with an EOF/incomplete-body error, got {err:?}" + ); + + let read_err = match set_disks + .get_object_reader(bucket, &object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("{name} short failed put must not leave a readable object"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&read_err) || matches!(read_err, Error::ErasureReadQuorum), + "{name} short failed put must fail closed on read, got {read_err:?}" + ); + } + } + + #[tokio::test] + async fn set_level_get_restores_body_when_one_shard_is_missing_after_write() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-read-repair-missing-shard"; + let object = "object.bin"; + let payload = (0..(BLOCK_SIZE_V2 + 211)) + .map(|idx| ((idx * 13) % 251) as u8) + .collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written before shard loss"); + + { + let mut disks = set_disks.disks.write().await; + disks[1] = None; + } + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("object should remain readable with one missing shard"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("degraded read should stream"); + + assert_eq!(restored, payload); + } + + #[tokio::test] + async fn set_level_overwrite_restores_new_body_and_cleans_old_data_dir() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-object-overwrite"; + let object = "nested/object.bin"; + let first_payload = (0..(BLOCK_SIZE_V2 + 31)).map(|idx| (idx % 239) as u8).collect::>(); + let second_payload = (0..(BLOCK_SIZE_V2 + 97)) + .map(|idx| ((idx * 7) % 251) as u8) + .collect::>(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut first_reader = PutObjReader::from_vec(first_payload); + set_disks + .put_object(bucket, object, &mut first_reader, &opts) + .await + .expect("first object body should be written"); + + let mut second_reader = PutObjReader::from_vec(second_payload.clone()); + let second = set_disks + .put_object(bucket, object, &mut second_reader, &opts) + .await + .expect("overwrite body should commit and clean the old data dir"); + + assert_eq!(second.size, second_payload.len() as i64); + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("overwritten object reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("overwritten object should stream"); + assert_eq!(restored, second_payload); + } + + #[tokio::test] + async fn set_level_write_preconditions_fail_closed_and_allow_matching_etags() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-write-preconditions"; + let object = "object.txt"; + let write_opts = ObjectOptions { + no_lock: true, + preserve_etag: Some("conditional-etag".to_string()), + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"conditional body".to_vec()); + set_disks + .put_object(bucket, object, &mut reader, &write_opts) + .await + .expect("object should be written with deterministic etag"); + + let reject_existing = ObjectOptions { + http_preconditions: Some(HTTPPreconditions { + if_none_match: Some("conditional-etag".to_string()), + ..Default::default() + }), + ..Default::default() + }; + assert!(matches!( + set_disks.check_write_precondition(bucket, object, &reject_existing).await, + Some(StorageError::PreconditionFailed) + )); + + let allow_matching = ObjectOptions { + http_preconditions: Some(HTTPPreconditions { + if_match: Some("\"conditional-etag\"".to_string()), + ..Default::default() + }), + ..Default::default() + }; + assert!( + set_disks + .check_write_precondition(bucket, object, &allow_matching) + .await + .is_none() + ); + + let missing_if_match = ObjectOptions { + http_preconditions: Some(HTTPPreconditions { + if_match: Some("missing-etag".to_string()), + ..Default::default() + }), + ..Default::default() + }; + assert!(matches!( + set_disks + .check_write_precondition(bucket, "missing-object.txt", &missing_if_match) + .await, + Some(StorageError::ObjectNotFound(_, _)) + )); + } + + #[tokio::test] + async fn set_level_versioned_delete_marker_hides_object_without_corrupting_version_metadata() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-versioned-delete"; + let object = "object.txt"; + let opts = ObjectOptions { + no_lock: true, + versioned: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"versioned object body".to_vec()); + let written = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("versioned object should be written"); + assert!(written.version_id.is_some()); + + let marker = set_disks + .delete_object(bucket, object, opts.clone()) + .await + .expect("versioned delete should create a marker"); + + assert!(marker.delete_marker); + assert!(marker.version_id.is_some()); + let err = match set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("latest delete marker must hide object body"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&err) || matches!(err, Error::MethodNotAllowed), + "delete marker read must fail closed, got {err:?}" + ); + } + + #[tokio::test] + async fn set_level_metadata_self_copy_preserves_body_and_updates_metadata() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-metadata-copy"; + let object = "object.txt"; + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + let payload = b"metadata copy must not lose committed bytes".to_vec(); + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written"); + + let mut source = set_disks + .get_object_info(bucket, object, &opts) + .await + .expect("object info should be readable"); + let mut metadata = (*source.user_defined).clone(); + metadata.insert("x-amz-meta-copy-check".to_string(), "present".to_string()); + source.user_defined = Arc::new(metadata); + source.metadata_only = true; + source.etag = Some("metadata-copy-etag".to_string()); + + let copied = set_disks + .copy_object(bucket, object, bucket, object, &mut source, &opts, &opts) + .await + .expect("metadata self-copy should succeed"); + assert_eq!(copied.user_defined.get("x-amz-meta-copy-check").map(String::as_str), Some("present")); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("copied object reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("copied object should stream"); + assert_eq!(restored, payload); + + let reread = set_disks + .get_object_info(bucket, object, &opts) + .await + .expect("copied object info should be readable"); + assert_eq!(reread.user_defined.get("x-amz-meta-copy-check").map(String::as_str), Some("present")); + } + + #[tokio::test] + async fn set_level_empty_object_read_uses_buffered_empty_body_with_locks() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-empty-object"; + let object = "empty.bin"; + let opts = ObjectOptions::default(); + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(Vec::new()); + let written = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("empty object should be written"); + assert_eq!(written.size, 0); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("empty object reader should open"); + assert_eq!(get_reader.object_info.size, 0); + assert_eq!(get_reader.buffered_body.as_ref().map(Bytes::len), Some(0)); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("empty object should stream"); + assert!(restored.is_empty()); + + let info = set_disks + .get_object_info(bucket, object, &opts) + .await + .expect("empty object info should be readable"); + assert_eq!(info.size, 0); + } + + #[tokio::test] + async fn set_level_put_object_options_preserve_etag_and_normalize_standard_storage_class() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-put-options"; + let object = "object.txt"; + let mod_time = OffsetDateTime::from_unix_timestamp(1_717_171_717).expect("fixed timestamp should parse"); + let mut user_defined = HashMap::new(); + user_defined.insert(AMZ_STORAGE_CLASS.to_string(), storageclass::STANDARD.to_string()); + user_defined.insert(SUFFIX_COMPRESSION.to_string(), "zstd".to_string()); + let mut eval_metadata = HashMap::new(); + eval_metadata.insert("x-amz-meta-evaluated".to_string(), "yes".to_string()); + let opts = ObjectOptions { + mod_time: Some(mod_time), + preserve_etag: Some("preserved-etag".to_string()), + user_defined, + eval_metadata: Some(eval_metadata), + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"option matrix body".to_vec()); + let written = set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("object should be written with option matrix"); + assert_eq!(written.etag.as_deref(), Some("preserved-etag")); + assert_eq!(written.mod_time, Some(mod_time)); + assert_eq!(written.user_defined.get("x-amz-meta-evaluated").map(String::as_str), Some("yes")); + assert!(!written.user_defined.contains_key(AMZ_STORAGE_CLASS)); + + let info = set_disks + .get_object_info(bucket, object, &opts) + .await + .expect("object info should be readable"); + assert_eq!(info.etag.as_deref(), Some("preserved-etag")); + assert_eq!(info.mod_time, Some(mod_time)); + assert_eq!(info.user_defined.get("x-amz-meta-evaluated").map(String::as_str), Some("yes")); + assert!(!info.user_defined.contains_key(AMZ_STORAGE_CLASS)); + } + + #[tokio::test] + async fn set_level_put_object_metadata_updates_headers_without_rewriting_body() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-put-metadata"; + let object = "object.txt"; + let payload = b"metadata update must preserve bytes".to_vec(); + let write_opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(payload.clone()); + set_disks + .put_object(bucket, object, &mut reader, &write_opts) + .await + .expect("object should be written"); + + let mut eval_metadata = HashMap::new(); + eval_metadata.insert("x-amz-meta-updated".to_string(), "true".to_string()); + let update_time = OffsetDateTime::from_unix_timestamp(1_717_181_818).expect("fixed timestamp should parse"); + let update_opts = ObjectOptions { + eval_metadata: Some(eval_metadata), + mod_time: Some(update_time), + ..Default::default() + }; + let updated = set_disks + .put_object_metadata(bucket, object, &update_opts) + .await + .expect("metadata update should succeed"); + assert_eq!(updated.mod_time, Some(update_time)); + assert_eq!(updated.user_defined.get("x-amz-meta-updated").map(String::as_str), Some("true")); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &write_opts) + .await + .expect("updated object reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("updated object should stream"); + assert_eq!(restored, payload); + } + + #[tokio::test] + async fn set_level_copy_object_with_prefetched_reader_restores_body() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-copy-reader"; + let object = "object.txt"; + let payload = b"copy reader body".to_vec(); + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut source = ObjectInfo { + metadata_only: false, + put_object_reader: Some(PutObjReader::from_vec(payload.clone())), + ..Default::default() + }; + let copied = set_disks + .copy_object(bucket, object, bucket, object, &mut source, &opts, &opts) + .await + .expect("copy with prefetched reader should write object data"); + assert_eq!(copied.size, payload.len() as i64); + + let mut get_reader = set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + .expect("copied reader should open"); + let mut restored = Vec::new(); + get_reader + .stream + .read_to_end(&mut restored) + .await + .expect("copied object should stream"); + assert_eq!(restored, payload); + } + + #[tokio::test] + async fn set_level_version_suspended_delete_creates_null_delete_marker() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-version-suspended-delete"; + let object = "object.txt"; + let opts = ObjectOptions { + no_lock: true, + version_suspended: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"suspended version body".to_vec()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("suspended-version object should be written"); + + let marker = set_disks + .delete_object(bucket, object, opts.clone()) + .await + .expect("version-suspended delete should create a null marker"); + assert!(marker.delete_marker); + assert_eq!(marker.version_id, Some(Uuid::nil())); + + let err = match set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("null delete marker must hide object body"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&err) || matches!(err, Error::MethodNotAllowed), + "null delete marker read must fail closed, got {err:?}" + ); + } + + #[tokio::test] + async fn set_level_delete_prefix_removes_nested_objects() { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-delete-prefix"; + let object = "prefix/object.txt"; + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"prefix body".to_vec()); + set_disks + .put_object(bucket, object, &mut reader, &opts) + .await + .expect("prefix object should be written"); + + set_disks + .delete_object( + bucket, + "prefix/", + ObjectOptions { + delete_prefix: true, + no_lock: true, + ..Default::default() + }, + ) + .await + .expect("prefix delete should succeed"); + + let err = match set_disks + .get_object_reader(bucket, object, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("object under deleted prefix must not be readable"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&err), + "prefix-deleted object must fail closed with object-not-found, got {err:?}" + ); + } + + #[tokio::test] + #[serial] + async fn set_level_delete_objects_batch_removes_live_data_and_tolerates_missing_keys() { + temp_env::async_with_vars([("RUSTFS_ISSUE3031_DIAG_ENABLE", Some("true"))], async { + let set_disks = make_local_bucket_test_set_disks().await; + let bucket = "bucket-delete-objects"; + let existing = "existing.txt"; + let missing = "missing.txt"; + let opts = ObjectOptions { + no_lock: true, + ..Default::default() + }; + + set_disks + .make_bucket(bucket, &MakeBucketOptions::default()) + .await + .expect("bucket should be created"); + let mut reader = PutObjReader::from_vec(b"batch delete body".to_vec()); + set_disks + .put_object(bucket, existing, &mut reader, &opts) + .await + .expect("object should be written before batch delete"); + + let (deleted, errors) = set_disks + .delete_objects( + bucket, + vec![ + ObjectToDelete { + object_name: existing.to_string(), + ..Default::default() + }, + ObjectToDelete { + object_name: missing.to_string(), + ..Default::default() + }, + ], + opts.clone(), + ) + .await; + + assert_eq!(deleted.len(), 2); + assert_eq!(errors.len(), 2); + assert!(errors.iter().all(Option::is_none)); + assert_eq!(deleted[0].object_name, existing); + assert!(deleted[0].found); + assert!(!deleted[0].delete_marker); + assert_eq!(deleted[1].object_name, missing); + assert!(!deleted[1].found); + assert!(!deleted[1].delete_marker); + + let err = match set_disks + .get_object_reader(bucket, existing, None, HeaderMap::new(), &opts) + .await + { + Ok(_) => panic!("batch-deleted object must not be readable as latest"), + Err(err) => err, + }; + assert!( + is_err_object_not_found(&err), + "batch-deleted object must fail closed with object-not-found, got {err:?}" + ); + }) + .await; + } + #[tokio::test] async fn set_level_heal_format_repairs_unformatted_disk() { let set_disks = make_local_bucket_test_set_disks_with_missing_format().await; diff --git a/crates/ecstore/src/set_disk/read.rs b/crates/ecstore/src/set_disk/read.rs index 66a6e2bf7..7b5e8cad4 100644 --- a/crates/ecstore/src/set_disk/read.rs +++ b/crates/ecstore/src/set_disk/read.rs @@ -1641,6 +1641,21 @@ mod metadata_cache_tests { .await } + async fn new_read_version_test_disk(bucket: &str) -> (tempfile::TempDir, DiskStore) { + let dir = tempfile::tempdir().expect("temp dir should be created"); + let endpoint = + crate::layout::endpoint::Endpoint::try_from(dir.path().to_string_lossy().as_ref()).expect("endpoint should parse"); + let local_disk = crate::disk::local::LocalDisk::new(&endpoint, false) + .await + .expect("local disk should open"); + let disk: DiskStore = Arc::new(crate::disk::Disk::Local(Box::new(crate::disk::disk_store::LocalDiskWrapper::new( + Arc::new(local_disk), + false, + )))); + disk.make_volume(bucket).await.expect("bucket volume should be created"); + (dir, disk) + } + #[test] #[serial] fn get_object_metadata_cache_capacity_uses_default_and_env_override() { @@ -1667,6 +1682,249 @@ mod metadata_cache_tests { fi } + #[tokio::test] + async fn get_object_with_fileinfo_rejects_invalid_ranges_before_reader_setup() { + let bucket = "bucket"; + let object = "object"; + + let mut output = Vec::new(); + let err = SetDisks::get_object_with_fileinfo( + bucket, + object, + 2, + 1, + &mut output, + valid_test_fileinfo(object), + Vec::new(), + &[], + 0, + 0, + false, + false, + GET_OBJECT_PATH_SET_DISK, + "plain", + "small", + ) + .await + .expect_err("offset beyond object size must fail before reader setup"); + assert!(err.to_string().contains("offset out of range")); + + let mut overflow = valid_test_fileinfo(object); + overflow.size = -1; + let err = SetDisks::get_object_with_fileinfo( + bucket, + object, + usize::MAX, + 1, + &mut output, + overflow, + Vec::new(), + &[], + 0, + 0, + false, + false, + GET_OBJECT_PATH_SET_DISK, + "plain", + "small", + ) + .await + .expect_err("offset plus length overflow must fail before reader setup"); + assert!(err.to_string().contains("offset out of range")); + + let err = SetDisks::get_object_with_fileinfo( + bucket, + object, + 1, + 1, + &mut output, + valid_test_fileinfo(object), + Vec::new(), + &[], + 0, + 0, + false, + false, + GET_OBJECT_PATH_SET_DISK, + "plain", + "small", + ) + .await + .expect_err("end offset beyond object size must fail before reader setup"); + assert!(err.to_string().contains("offset out of range")); + + let mut invalid_erasure = valid_test_fileinfo(object); + invalid_erasure.erasure.block_size = 0; + let err = SetDisks::get_object_with_fileinfo( + bucket, + object, + 0, + 1, + &mut output, + invalid_erasure, + Vec::new(), + &[], + 0, + 0, + false, + false, + GET_OBJECT_PATH_SET_DISK, + "plain", + "small", + ) + .await + .expect_err("invalid erasure metadata must fail before reader setup"); + assert!(err.to_string().contains("invalid erasure metadata")); + assert!(output.is_empty()); + } + + #[tokio::test] + async fn get_object_with_fileinfo_fails_closed_without_read_quorum() { + let bucket = "bucket"; + let object = "object"; + let mut fi = valid_test_fileinfo(object); + fi.erasure.block_size = 1; + fi.erasure.distribution = vec![1, 2, 3, 4]; + fi.parts.push(ObjectPartInfo { + number: 1, + size: 1, + actual_size: 1, + ..Default::default() + }); + + let mut output = Vec::new(); + let err = SetDisks::get_object_with_fileinfo( + bucket, + object, + 0, + 1, + &mut output, + fi, + Vec::new(), + &[], + 0, + 0, + false, + false, + GET_OBJECT_PATH_SET_DISK, + "plain", + "small", + ) + .await + .expect_err("object read must fail closed when no shards can be read"); + + let err = err.to_string().to_ascii_lowercase(); + assert!(!err.is_empty(), "read should fail with a concrete error"); + assert!(output.is_empty()); + } + + #[tokio::test] + async fn read_version_optimized_reads_local_metadata_and_fails_closed_without_quorum() { + let bucket = "read-version-optimized-bucket"; + let object = "object"; + let (_dir, disk) = new_read_version_test_disk(bucket).await; + let mut fi = valid_test_fileinfo(object); + fi.mod_time = Some(OffsetDateTime::now_utc()); + disk.write_metadata(bucket, bucket, object, fi.clone()) + .await + .expect("metadata should be written before optimized read"); + + let set = SetDisks::new( + "read-version-optimized-test".to_string(), + Arc::new(RwLock::new(vec![Some(disk)])), + 1, + 0, + 0, + 0, + Vec::new(), + FormatV3::new(1, 1), + Vec::new(), + ) + .await; + + let versions = set + .read_version_optimized(bucket, object, "", &ReadOptions::default()) + .await + .expect("single readable disk should satisfy optimized read quorum"); + assert_eq!(versions.len(), 1); + assert_eq!(versions[0].name, object); + assert_eq!(versions[0].metadata.get("etag").map(String::as_str), Some("etag-1")); + + let missing_quorum = new_metadata_cache_test_set() + .await + .read_version_optimized(bucket, object, "", &ReadOptions::default()) + .await + .expect_err("empty disk set must fail closed"); + assert!( + missing_quorum.to_string().to_ascii_lowercase().contains("file"), + "optimized read failure should map to file-not-found style error: {missing_quorum}" + ); + } + + #[tokio::test] + async fn get_object_info_and_quorum_maps_delete_marker_and_purge_states() { + let bucket = "get-object-info-marker-bucket"; + let (_dir, disk) = new_read_version_test_disk(bucket).await; + let set = SetDisks::new( + "get-object-info-marker-test".to_string(), + Arc::new(RwLock::new(vec![Some(disk.clone())])), + 1, + 0, + 0, + 0, + Vec::new(), + FormatV3::new(1, 1), + Vec::new(), + ) + .await; + + let latest_marker = FileInfo { + volume: bucket.to_string(), + name: "latest-delete-marker".to_string(), + version_id: Some(Uuid::new_v4()), + deleted: true, + mod_time: Some(OffsetDateTime::now_utc()), + ..Default::default() + }; + disk.write_metadata(bucket, bucket, "latest-delete-marker", latest_marker) + .await + .expect("latest marker metadata should be written"); + let (_, _, latest_err) = set + .get_object_info_and_quorum(bucket, "latest-delete-marker", &ObjectOptions::default()) + .await; + assert!( + matches!(latest_err, Some(StorageError::ObjectNotFound(_, _))), + "latest delete marker should hide the object, got {latest_err:?}" + ); + + let marker_version = Uuid::new_v4(); + let version_marker = FileInfo { + volume: bucket.to_string(), + name: "version-delete-marker".to_string(), + version_id: Some(marker_version), + deleted: true, + mod_time: Some(OffsetDateTime::now_utc()), + ..Default::default() + }; + disk.write_metadata(bucket, bucket, "version-delete-marker", version_marker) + .await + .expect("version marker metadata should be written"); + let (_, _, version_err) = set + .get_object_info_and_quorum( + bucket, + "version-delete-marker", + &ObjectOptions { + version_id: Some(marker_version.to_string()), + ..Default::default() + }, + ) + .await; + assert!( + matches!(version_err, Some(StorageError::MethodNotAllowed)), + "explicit delete marker read should be method-not-allowed, got {version_err:?}" + ); + } + #[test] fn get_object_metadata_cache_request_eligibility_is_conservative() { let opts = ObjectOptions::default(); @@ -2055,6 +2313,26 @@ mod tests { const CODEC_STREAMING_TEST_BUCKET: &str = "bucket"; const CODEC_STREAMING_TEST_OBJECT: &str = "object"; + async fn local_test_disks(count: usize, bucket: &str) -> (Vec, Vec>) { + let mut dirs = Vec::with_capacity(count); + let mut disks = Vec::with_capacity(count); + for _ in 0..count { + let dir = tempfile::tempdir().expect("temp dir should be created"); + let endpoint = crate::layout::endpoint::Endpoint::try_from(dir.path().to_string_lossy().as_ref()) + .expect("endpoint should parse"); + let local_disk = crate::disk::local::LocalDisk::new(&endpoint, false) + .await + .expect("local disk should open"); + let disk: crate::disk::DiskStore = Arc::new(crate::disk::Disk::Local(Box::new( + crate::disk::disk_store::LocalDiskWrapper::new(Arc::new(local_disk), false), + ))); + disk.make_volume(bucket).await.expect("bucket volume should be created"); + dirs.push(dir); + disks.push(Some(disk)); + } + (dirs, disks) + } + #[test] fn shard_read_cost_for_endpoint_maps_topology_classes() { let local_hosts = vec!["node-a:9000".to_string()]; @@ -2772,6 +3050,299 @@ mod tests { assert!(result.is_err(), "zero block_size metadata must be rejected, not panic"); } + #[tokio::test] + async fn codec_streaming_fileinfo_rejects_invalid_size_and_multipart_mismatch() { + let mut single_part = codec_streaming_test_fileinfo(8, 1); + single_part.size = -1; + let invalid_size = SetDisks::get_object_decode_reader_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &single_part, + &[], + &[], + 0, + 0, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await; + assert!(invalid_size.is_err(), "negative object size must reject before reader construction"); + + let mut multipart = codec_streaming_test_fileinfo(17, 2); + multipart.parts[0].size = 8; + multipart.parts[1].size = 8; + let mismatch = temp_env::async_with_vars([(ENV_RUSTFS_GET_CODEC_STREAMING_MULTIPART_ENABLE, Some("true"))], async { + SetDisks::get_object_decode_reader_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &multipart, + &[], + &[], + 0, + 0, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await + }) + .await; + assert!(mismatch.is_err(), "multipart part sizes must match object size"); + } + + #[tokio::test] + async fn codec_streaming_fileinfo_reports_multipart_fallbacks_at_entry() { + let multipart = codec_streaming_test_fileinfo(16, 2); + + let disabled = temp_env::async_with_vars([(ENV_RUSTFS_GET_CODEC_STREAMING_MULTIPART_ENABLE, Some("false"))], async { + SetDisks::get_object_decode_reader_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &multipart, + &[], + &[], + 0, + 0, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await + }) + .await + .expect("multipart disabled should return a structured fallback"); + assert!(matches!( + disabled, + GetCodecStreamingReaderBuildOutcome::Fallback(GetCodecStreamingFallbackReason::Multipart) + )); + + let part_limit = temp_env::async_with_vars( + [ + (ENV_RUSTFS_GET_CODEC_STREAMING_MULTIPART_ENABLE, Some("true")), + (ENV_RUSTFS_GET_CODEC_STREAMING_MULTIPART_MAX_PARTS, Some("1")), + ], + async { + SetDisks::get_object_decode_reader_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &multipart, + &[], + &[], + 0, + 0, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await + }, + ) + .await + .expect("multipart part limit should return a structured fallback"); + assert!(matches!( + part_limit, + GetCodecStreamingReaderBuildOutcome::Fallback(GetCodecStreamingFallbackReason::MultipartPartLimit) + )); + } + + #[tokio::test] + async fn codec_streaming_fileinfo_builds_lazy_multipart_reader_from_inline_shards() { + let part_data = b"abcdefgh"; + let erasure = coding::Erasure::new(4, 2, part_data.len()); + let mut fi = codec_streaming_test_fileinfo(16, 2); + fi.erasure.block_size = part_data.len(); + fi.erasure.distribution = (1..=erasure.total_shard_count()).collect(); + for part in &mut fi.parts { + part.size = part_data.len(); + part.actual_size = i64::try_from(part_data.len()).expect("test part size should fit i64"); + } + let files = codec_streaming_inline_files(&erasure, part_data).await; + let dir = tempfile::tempdir().expect("temp dir should be created"); + let endpoint = + crate::layout::endpoint::Endpoint::try_from(dir.path().to_string_lossy().as_ref()).expect("endpoint should parse"); + let local_disk = crate::disk::local::LocalDisk::new(&endpoint, false) + .await + .expect("local disk should open"); + let disk: crate::disk::DiskStore = Arc::new(crate::disk::Disk::Local(Box::new( + crate::disk::disk_store::LocalDiskWrapper::new(Arc::new(local_disk), false), + ))); + let disks = vec![Some(disk); files.len()]; + + let outcome = temp_env::async_with_vars( + [ + (ENV_RUSTFS_GET_CODEC_STREAMING_MULTIPART_ENABLE, Some("true")), + (ENV_RUSTFS_GET_CODEC_STREAMING_MULTIPART_MAX_PARTS, Some("4")), + ], + async { + SetDisks::get_object_decode_reader_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &fi, + &files, + &disks, + 0, + 0, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await + }, + ) + .await + .expect("lazy multipart reader should be constructed"); + + let GetCodecStreamingReaderBuildOutcome::Reader(mut reader) = outcome else { + panic!("expected lazy multipart reader"); + }; + let mut body = Vec::new(); + reader + .read_to_end(&mut body) + .await + .expect("lazy multipart reader should restore both parts"); + assert_eq!(body, [part_data.as_slice(), part_data.as_slice()].concat()); + } + + #[tokio::test] + #[serial_test::serial] + async fn codec_streaming_fileinfo_builds_single_part_reader_from_inline_shards() { + let part_data = b"abcdefgh"; + let erasure = coding::Erasure::new(4, 2, part_data.len()); + let mut fi = codec_streaming_test_fileinfo(part_data.len() as i64, 1); + fi.erasure.block_size = part_data.len(); + fi.erasure.distribution = (1..=erasure.total_shard_count()).collect(); + fi.parts[0].size = part_data.len(); + fi.parts[0].actual_size = i64::try_from(part_data.len()).expect("test part size should fit i64"); + let files = codec_streaming_inline_files(&erasure, part_data).await; + let dir = tempfile::tempdir().expect("temp dir should be created"); + let endpoint = + crate::layout::endpoint::Endpoint::try_from(dir.path().to_string_lossy().as_ref()).expect("endpoint should parse"); + let local_disk = crate::disk::local::LocalDisk::new(&endpoint, false) + .await + .expect("local disk should open"); + let disk: crate::disk::DiskStore = Arc::new(crate::disk::Disk::Local(Box::new( + crate::disk::disk_store::LocalDiskWrapper::new(Arc::new(local_disk), false), + ))); + let disks = vec![Some(disk); files.len()]; + + let outcome = temp_env::async_with_vars([("RUSTFS_SHARD_LOCALITY_SCHEDULING", Some("on"))], async { + SetDisks::get_object_decode_reader_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &fi, + &files, + &disks, + 0, + 0, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await + }) + .await + .expect("single part reader should be constructed"); + + let GetCodecStreamingReaderBuildOutcome::Reader(mut reader) = outcome else { + panic!("expected single part reader"); + }; + let mut body = Vec::new(); + reader + .read_to_end(&mut body) + .await + .expect("single part codec reader should restore payload"); + assert_eq!(body, part_data); + } + + #[tokio::test] + async fn get_object_with_fileinfo_restores_missing_inline_data_shard_and_submits_repair() { + let part_data = b"abcdefgh"; + let erasure = coding::Erasure::new(4, 2, part_data.len()); + let mut fi = codec_streaming_test_fileinfo(part_data.len() as i64, 1); + fi.erasure.block_size = part_data.len(); + fi.erasure.distribution = (1..=erasure.total_shard_count()).collect(); + fi.parts[0].size = part_data.len(); + fi.parts[0].actual_size = i64::try_from(part_data.len()).expect("test part size should fit i64"); + + let mut files = codec_streaming_inline_files(&erasure, part_data).await; + files[0].data = None; + let (_dirs, disks) = local_test_disks(files.len(), CODEC_STREAMING_TEST_BUCKET).await; + + let mut output = Vec::new(); + SetDisks::get_object_with_fileinfo( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + 0, + part_data.len() as i64, + &mut output, + fi, + files, + &disks, + 0, + 0, + false, + false, + GET_OBJECT_PATH_SET_DISK, + "test-object-class", + "test-size-bucket", + ) + .await + .expect("missing data shard should be reconstructed from parity"); + + assert_eq!(output, part_data); + } + + #[tokio::test] + async fn codec_streaming_part_reader_rejects_oversized_part_and_missing_quorum() { + let erasure = coding::Erasure::new(4, 2, 8); + let fi = codec_streaming_test_fileinfo(8, 1); + + let oversized = SetDisks::build_codec_streaming_part_reader( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &fi, + &[], + &[], + &erasure, + 1, + 0, + 9, + 8, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await; + assert!(oversized.is_err(), "part_length > part_size must be rejected"); + + let missing_quorum = SetDisks::build_codec_streaming_part_reader( + CODEC_STREAMING_TEST_BUCKET, + CODEC_STREAMING_TEST_OBJECT, + &fi, + &[], + &[], + &erasure, + 1, + 0, + 8, + 8, + false, + "test-object-class", + "test-size-bucket", + false, + ) + .await; + assert!(missing_quorum.is_err(), "reader setup must fail closed when no shard can answer"); + } + #[tokio::test] async fn multipart_codec_streaming_reader_reads_parts_in_order() { let readers: Vec> = vec![ @@ -3190,6 +3761,29 @@ mod tests { Ok(decoded) } + async fn codec_streaming_inline_files(erasure: &coding::Erasure, part_data: &'static [u8]) -> Vec { + let shards = erasure.encode_data(part_data).expect("test part should encode"); + let distribution = (1..=erasure.total_shard_count()).collect::>(); + let mut files = Vec::with_capacity(shards.len()); + + for shard in shards { + let mut writer = BitrotWriter::new(Cursor::new(Vec::new()), erasure.shard_size(), HashAlgorithm::HighwayHash256S); + writer.write(&shard).await.expect("test shard should write with bitrot hash"); + + let mut fi = FileInfo::new(CODEC_STREAMING_TEST_OBJECT, erasure.data_shards, erasure.parity_shards); + fi.volume = CODEC_STREAMING_TEST_BUCKET.to_string(); + fi.name = CODEC_STREAMING_TEST_OBJECT.to_string(); + fi.size = i64::try_from(part_data.len()).expect("test part size should fit i64"); + fi.erasure.block_size = erasure.block_size; + fi.erasure.index = files.len() + 1; + fi.erasure.distribution = distribution.clone(); + fi.data = Some(Bytes::from(writer.into_inner().into_inner())); + files.push(fi); + } + + files + } + #[tokio::test] async fn bitrot_reader_setup_stops_at_read_quorum() { let setup = setup_inline_bitrot_readers( diff --git a/crates/ecstore/src/set_disk/shard_source.rs b/crates/ecstore/src/set_disk/shard_source.rs index 34b3336a6..4e497149a 100644 --- a/crates/ecstore/src/set_disk/shard_source.rs +++ b/crates/ecstore/src/set_disk/shard_source.rs @@ -240,6 +240,18 @@ mod tests { assert_eq!(ShardReadCost::SameNode.as_str(), GET_SHARD_READ_COST_SAME_NODE); } + #[test] + fn shard_read_cost_reports_all_labels_and_remote_classification() { + assert_eq!(ShardReadCost::Local.as_str(), GET_SHARD_READ_COST_LOCAL); + assert_eq!(ShardReadCost::SameNode.as_str(), GET_SHARD_READ_COST_SAME_NODE); + assert_eq!(ShardReadCost::Remote.as_str(), GET_SHARD_READ_COST_REMOTE); + assert_eq!(ShardReadCost::Unknown.as_str(), GET_SHARD_READ_COST_UNKNOWN); + + assert!(ShardReadCost::Remote.is_remote()); + assert!(!ShardReadCost::Local.is_remote()); + assert!(!ShardReadCost::Unknown.is_low_cost()); + } + #[test] fn stripe_read_state_reports_complete_data_shards_without_parity() { let state = StripeReadState::from_parts(vec![Some(vec![1]), Some(vec![2]), None], Vec::new(), 2); diff --git a/crates/ecstore/src/store/bucket.rs b/crates/ecstore/src/store/bucket.rs index 27c28b93e..a0401a5ee 100644 --- a/crates/ecstore/src/store/bucket.rs +++ b/crates/ecstore/src/store/bucket.rs @@ -430,6 +430,18 @@ mod tests { false } + async fn write_bucket_metadata_marker(disk_paths: &[PathBuf], metadata_prefix: &str) { + for disk_path in disk_paths { + let marker_path = disk_path.join(metadata_prefix).join("config.json"); + tokio::fs::create_dir_all(marker_path.parent().expect("metadata marker path should have a parent")) + .await + .expect("metadata marker parent should be created"); + tokio::fs::write(marker_path, b"bucket metadata") + .await + .expect("metadata marker should be written"); + } + } + #[test] fn should_not_override_when_metadata_created_is_unix_epoch() { assert!(!should_override_created_from_metadata(OffsetDateTime::UNIX_EPOCH)); @@ -509,6 +521,7 @@ mod tests { let metadata_prefix = format!("{RUSTFS_META_BUCKET}/{BUCKET_META_PREFIX}/{bucket}"); create_bucket_with_object(&ecstore, &bucket, object).await; + write_bucket_metadata_marker(&disk_paths, &metadata_prefix).await; assert!(any_disk_path_exists(&disk_paths, &metadata_prefix).await); ecstore diff --git a/scripts/run_ecstore_validation_suite.sh b/scripts/run_ecstore_validation_suite.sh index e9a5532af..7771d21c7 100755 --- a/scripts/run_ecstore_validation_suite.sh +++ b/scripts/run_ecstore_validation_suite.sh @@ -192,7 +192,13 @@ run_core_unit_steps() { run_step "ecstore-rename-rollback" \ cargo test -p rustfs-ecstore --lib set_disk::tests::test_rename_data_quorum_failure_rolls_back_destination_object run_step "ecstore-disk-local-lib" cargo test -p rustfs-ecstore --lib disk::local - run_step "ecstore-lib-all" cargo test -p rustfs-ecstore --lib -- --test-threads=1 + run_step "ecstore-global-bucket-migration" \ + cargo test -p rustfs-ecstore --lib bucket::migration::tests::migrates_real_minio_bucket_metadata_end_to_end -- --exact + run_step "ecstore-global-delete-lock-gating" \ + cargo test -p rustfs-ecstore --lib set_disk::ops::object::delete_objects_lock_gating_tests::delete_objects_blocks_locked_object_and_deletes_the_rest -- --exact + run_step "ecstore-lib-all" cargo test -p rustfs-ecstore --lib -- --test-threads=1 \ + --skip bucket::migration::tests::migrates_real_minio_bucket_metadata_end_to_end \ + --skip set_disk::ops::object::delete_objects_lock_gating_tests::delete_objects_blocks_locked_object_and_deletes_the_rest } run_quick_e2e_steps() { @@ -436,7 +442,11 @@ run_coverage() { local lcov_path="$coverage_dir/lcov.info" local coverage_summary="$coverage_dir/summary.tsv" local coverage_files="$coverage_dir/files.tsv" - local cmd=(cargo llvm-cov -p rustfs-ecstore --lib --lcov --output-path "$lcov_path" -- --test-threads=1) + local cmd=( + cargo llvm-cov -p rustfs-ecstore --lib --lcov --output-path "$lcov_path" -- --test-threads=1 + --skip bucket::migration::tests::migrates_real_minio_bucket_metadata_end_to_end + --skip set_disk::ops::object::delete_objects_lock_gating_tests::delete_objects_blocks_locked_object_and_deletes_the_rest + ) local root root="$(pwd)" mkdir -p "$coverage_dir"