diff --git a/crates/config/src/constants/env.rs b/crates/config/src/constants/env.rs index aaa2ed02e..ccaf4697a 100644 --- a/crates/config/src/constants/env.rs +++ b/crates/config/src/constants/env.rs @@ -30,6 +30,16 @@ pub const COMMENT_KEY: &str = "comment"; pub const ENV_AUDIT_ENABLE: &str = "RUSTFS_AUDIT_ENABLE"; /// Global switch for enabling the notify module. pub const ENV_NOTIFY_ENABLE: &str = "RUSTFS_NOTIFY_ENABLE"; +/// Maximum audit entries per second for authorization denials of authenticated callers. +/// `0` disables denial auditing for that class of caller. +pub const ENV_AUDIT_DENIAL_RATE: &str = "RUSTFS_AUDIT_DENIAL_RATE"; +/// Maximum audit entries per second for authorization denials of anonymous callers. +/// `0` disables denial auditing for that class of caller. +pub const ENV_AUDIT_DENIAL_ANONYMOUS_RATE: &str = "RUSTFS_AUDIT_DENIAL_ANONYMOUS_RATE"; +/// Default denial audit rate for authenticated callers (entries per second). +pub const DEFAULT_AUDIT_DENIAL_RATE: u64 = 50; +/// Default denial audit rate for anonymous callers (entries per second). +pub const DEFAULT_AUDIT_DENIAL_ANONYMOUS_RATE: u64 = 5; /// Default global audit switch (disabled by default). pub const DEFAULT_AUDIT_ENABLE: bool = false; /// Default global notify switch (disabled by default). @@ -328,5 +338,7 @@ mod tests { fn test_global_audit_notify_switch_constants() { assert_eq!(ENV_AUDIT_ENABLE, "RUSTFS_AUDIT_ENABLE"); assert_eq!(ENV_NOTIFY_ENABLE, "RUSTFS_NOTIFY_ENABLE"); + assert_eq!(ENV_AUDIT_DENIAL_RATE, "RUSTFS_AUDIT_DENIAL_RATE"); + assert_eq!(ENV_AUDIT_DENIAL_ANONYMOUS_RATE, "RUSTFS_AUDIT_DENIAL_ANONYMOUS_RATE"); } } diff --git a/crates/ecstore/src/bucket/metadata.rs b/crates/ecstore/src/bucket/metadata.rs index 740c52d26..e8b432a5c 100644 --- a/crates/ecstore/src/bucket/metadata.rs +++ b/crates/ecstore/src/bucket/metadata.rs @@ -1503,7 +1503,11 @@ pub(crate) async fn load_bucket_metadata_parse_with_presence( bucket: &str, parse: bool, ) -> Result<(BucketMetadata, bool)> { - let (mut bm, persisted) = match read_bucket_metadata(api.clone(), bucket).await { + let (metadata_res, incarnation_res) = tokio::join!( + Box::pin(read_bucket_metadata(api.clone(), bucket)), + Box::pin(load_bucket_incarnation(api.clone(), bucket)), + ); + let (mut bm, persisted) = match metadata_res { Ok(res) => (res, true), Err(err) => { if err != Error::ConfigNotFound { @@ -1514,7 +1518,7 @@ pub(crate) async fn load_bucket_metadata_parse_with_presence( } }; - let incarnation = load_bucket_incarnation(api.clone(), bucket).await?; + let incarnation = incarnation_res?; if persisted { if bm.name.is_empty() { bm.name = bucket.to_string(); diff --git a/crates/ecstore/src/bucket/metadata_sys.rs b/crates/ecstore/src/bucket/metadata_sys.rs index 64425b155..46421b52e 100644 --- a/crates/ecstore/src/bucket/metadata_sys.rs +++ b/crates/ecstore/src/bucket/metadata_sys.rs @@ -1435,6 +1435,7 @@ pub(crate) async fn get_object_lock_config_state_in( /// Re-read the Object Lock state and bucket incarnation from the same /// authoritative metadata blob while the caller holds the transaction lock. +#[cfg(all(test, feature = "test-util"))] pub(crate) async fn get_object_lock_config_and_incarnation_from_disk_in( ctx: &crate::runtime::instance::InstanceContext, bucket: &str, @@ -1453,6 +1454,27 @@ pub(crate) async fn get_object_lock_config_and_incarnation_from_disk_with_guard_ get_delete_config_from_disk_in(ctx, bucket, guard, None).await } +#[allow(clippy::type_complexity)] +pub(crate) async fn get_object_lock_and_quota_config_from_disk_with_guard_in( + ctx: &crate::runtime::instance::InstanceContext, + bucket: &str, + guard: Option<&rustfs_lock::NamespaceLockGuard>, +) -> Result<( + ObjectLockConfigState, + Uuid, + OffsetDateTime, + std::result::Result<(Option, Uuid, OffsetDateTime), String>, +)> { + let (state, incarnation_id, updated_at, quota_res) = + get_delete_and_quota_config_from_disk_in(ctx, bucket, guard, None, true).await?; + Ok(( + state, + incarnation_id, + updated_at, + quota_res.unwrap_or(Ok((None, incarnation_id, updated_at))), + )) +} + /// Validate queued expiration against the same fresh metadata used for Object /// Lock admission. The caller holds the transaction read lock through commit, /// so a successful lifecycle update cannot be followed by an old-policy delete. @@ -1462,6 +1484,24 @@ pub(crate) async fn get_delete_config_from_disk_in( guard: Option<&rustfs_lock::NamespaceLockGuard>, expiry_policy: Option<&rustfs_lifecycle::ExpiryPolicySnapshot>, ) -> Result<(ObjectLockConfigState, Uuid, OffsetDateTime)> { + let (state, incarnation_id, updated_at, _) = + get_delete_and_quota_config_from_disk_in(ctx, bucket, guard, expiry_policy, false).await?; + Ok((state, incarnation_id, updated_at)) +} + +#[allow(clippy::type_complexity)] +async fn get_delete_and_quota_config_from_disk_in( + ctx: &crate::runtime::instance::InstanceContext, + bucket: &str, + guard: Option<&rustfs_lock::NamespaceLockGuard>, + expiry_policy: Option<&rustfs_lifecycle::ExpiryPolicySnapshot>, + include_quota: bool, +) -> Result<( + ObjectLockConfigState, + Uuid, + OffsetDateTime, + Option, Uuid, OffsetDateTime), String>>, +)> { let bucket_meta_sys_lock = bucket_metadata_sys_of(ctx)?; let bucket_meta_sys = bucket_meta_sys_lock.read().await.clone(); @@ -1477,7 +1517,7 @@ pub(crate) async fn get_delete_config_from_disk_in( .await? } }; - match authority { + match &authority { BucketMetadataAuthority::Authoritative(metadata) if metadata.bucket_incarnation_sidecar && !metadata.bucket_incarnation_id.is_nil() => { @@ -1489,11 +1529,11 @@ pub(crate) async fn get_delete_config_from_disk_in( } } let object_lock_config_updated_at = metadata.object_lock_config_updated_at; - Ok(( - object_lock_config_state_from_authoritative_metadata(&metadata)?, - metadata.bucket_incarnation_id, - object_lock_config_updated_at, - )) + let state = object_lock_config_state_from_authoritative_metadata(metadata)?; + let incarnation_id = metadata.bucket_incarnation_id; + let quota_res = include_quota + .then(|| quota_config_and_incarnation_from_authority(bucket, authority).map_err(|err| err.to_string())); + Ok((state, incarnation_id, object_lock_config_updated_at, quota_res)) } BucketMetadataAuthority::Authoritative(_) => { Err(Error::other(format!("bucket incarnation metadata is not authoritative: {bucket}"))) @@ -1564,6 +1604,23 @@ pub(crate) async fn get_quota_config_and_incarnation_from_disk_for_options_in( return Err(Error::other("quota snapshot metadata transaction fence was released")); } + #[cfg(test)] + let has_injected_error = sys.object_lock_disk_read_errors.read().await.contains(bucket); + #[cfg(not(test))] + let has_injected_error = false; + if !ctx.lock_manager().is_disabled() + && !has_injected_error + && let Some(fresh_quota) = snapshot.take_fresh_put_quota() + { + if snapshot_guard.is_lock_lost() + || snapshot_guard.is_released() + || !snapshot.is_valid_for_destructive_put(store.id, bucket, opts.expected_bucket_incarnation_id.unwrap_or_default()) + { + return Err(Error::other("quota snapshot fence or bucket incarnation changed during the disk read")); + } + return fresh_quota.map_err(Error::other); + } + // The snapshot carries the caller's lifecycle fence, excluding deletion. // Its transaction guard excludes config writes, so a same-name object // owner need not recursively acquire the bucket namespace here. @@ -1750,9 +1807,22 @@ struct MetadataPublishGuard { _guard: tokio::sync::OwnedMutexGuard, } +static LOCAL_META_MUTATION_GEN: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); + +pub(crate) fn bump_local_meta_mutation_gen() { + LOCAL_META_MUTATION_GEN.fetch_add(1, std::sync::atomic::Ordering::Release); +} + +pub(crate) fn current_local_meta_mutation_gen() -> u64 { + LOCAL_META_MUTATION_GEN.load(std::sync::atomic::Ordering::Acquire) +} + +type LocalAuthorityCache = Arc)>>>; + #[derive(Debug, Clone)] pub struct BucketMetadataSys { metadata_map: Arc>>>, + local_authority_cache: LocalAuthorityCache, /// Serializes metadata-map commits and their derived cache updates for one /// bucket. Namespace locks, when present, are acquired before this lock. metadata_publish_locks: Arc, @@ -1796,6 +1866,7 @@ impl BucketMetadataSys { pub fn new(api: Arc) -> Self { Self { metadata_map: Arc::new(RwLock::new(HashMap::new())), + local_authority_cache: Arc::new(StdMutex::new(HashMap::new())), metadata_publish_locks: Arc::new(MetadataPublishLockRegistry { locks: StdMutex::new(HashMap::new()), }), @@ -2160,6 +2231,7 @@ impl BucketMetadataSys { } pub async fn set(&self, bucket: String, bm: Arc) { + bump_local_meta_mutation_gen(); if !is_meta_bucketname(&bucket) { let publish_lock = self.metadata_publish_lock(&bucket); let _publish_guard = publish_lock.lock().await; @@ -2178,6 +2250,7 @@ impl BucketMetadataSys { /// /// Returns `true` if an entry was present. Reserved meta buckets are ignored. pub async fn remove(&self, bucket: &str) -> bool { + bump_local_meta_mutation_gen(); if is_meta_bucketname(bucket) { return false; } @@ -2198,6 +2271,7 @@ impl BucketMetadataSys { } async fn _reset(&mut self) { + bump_local_meta_mutation_gen(); let mut map = self.metadata_map.write().await; map.clear(); drop(map); @@ -3001,30 +3075,77 @@ impl BucketMetadataSys { return Err(Error::other(format!("injected Object Lock metadata disk read failure: {bucket}"))); } - match await_bucket_namespace_operation(Some(guard), bucket, "bucket metadata snapshot existence check", async { - self.object_store() - .get_bucket_info_from_sets_at_read_quorum(bucket, &crate::storage_api_contracts::bucket::BucketOptions::default()) - .await - }) - .await - { + let store = self.object_store(); + let can_use_local_authority_cache = !store.ctx.lock_manager().is_disabled() && store.all_disks_local_and_online(); + let gen_before = current_local_meta_mutation_gen(); + let cached_authority = can_use_local_authority_cache + .then(|| { + self.local_authority_cache + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(bucket) + .filter(|(cached_gen, _)| *cached_gen == gen_before) + .map(|(_, bm)| Arc::clone(bm)) + }) + .flatten(); + if let Some(cached) = cached_authority { + await_bucket_namespace_operation( + Some(guard), + bucket, + "bucket metadata authoritative snapshot", + std::future::ready(Ok::<_, Error>(())), + ) + .await?; + if current_local_meta_mutation_gen() == gen_before && store.all_disks_local_and_online() { + return Ok(BucketMetadataAuthority::Authoritative(cached)); + } + } + + let (bucket_info_res, metadata_res) = tokio::join!( + Box::pin(await_bucket_namespace_operation( + Some(guard), + bucket, + "bucket metadata snapshot existence check", + async { + store + .get_bucket_info_from_sets_at_read_quorum( + bucket, + &crate::storage_api_contracts::bucket::BucketOptions::default(), + ) + .await + }, + )), + Box::pin(await_bucket_namespace_operation( + Some(guard), + bucket, + "bucket metadata authoritative snapshot", + load_bucket_metadata_parse_with_presence(Arc::clone(&store), bucket, true), + )), + ); + match bucket_info_res { Ok(_) => {} Err(Error::VolumeNotFound) => return Ok(BucketMetadataAuthority::MissingBucket), Err(err) => return Err(err), } - let (metadata, persisted) = await_bucket_namespace_operation( - Some(guard), - bucket, - "bucket metadata authoritative snapshot", - load_bucket_metadata_parse_with_presence(self.object_store(), bucket, true), - ) - .await?; + let (metadata, persisted) = metadata_res?; if persisted && metadata.lock_enabled && !metadata.bucket_creation_committed { return Err(Error::ErasureWriteQuorum); } if persisted { - Ok(BucketMetadataAuthority::Authoritative(Arc::new(metadata))) + let metadata = Arc::new(metadata); + if can_use_local_authority_cache + && metadata.bucket_incarnation_sidecar + && !metadata.bucket_incarnation_id.is_nil() + && current_local_meta_mutation_gen() == gen_before + && store.all_disks_local_and_online() + { + self.local_authority_cache + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(bucket.to_string(), (gen_before, Arc::clone(&metadata))); + } + Ok(BucketMetadataAuthority::Authoritative(metadata)) } else { Ok(BucketMetadataAuthority::Fabricated) } diff --git a/crates/ecstore/src/crash_inject.rs b/crates/ecstore/src/crash_inject.rs index 30abe0d70..d759fd32a 100644 --- a/crates/ecstore/src/crash_inject.rs +++ b/crates/ecstore/src/crash_inject.rs @@ -84,7 +84,7 @@ pub(crate) enum CrashPoint { } #[cfg(test)] -pub(crate) use armed::{arm, disarm, should_crash_at}; +pub(crate) use armed::{arm, disarm, is_armed, should_crash_at}; /// Production no-op: the compiler inlines this to `false`, so armed crash points /// exist only in `#[cfg(test)]` builds and never add a branch to real commits. @@ -94,6 +94,12 @@ pub(crate) fn should_crash_at(_point: CrashPoint, _key: &str) -> bool { false } +#[cfg(not(test))] +#[inline(always)] +pub(crate) fn is_armed(_point: CrashPoint, _key: &str) -> bool { + false +} + #[cfg(test)] mod armed { use super::CrashPoint; @@ -132,6 +138,10 @@ mod armed { guard().retain(|(p, k)| !(*p == point && k == key)); } + pub(crate) fn is_armed(point: CrashPoint, key: &str) -> bool { + guard().iter().any(|(p, k)| *p == point && k == key) + } + /// Returns `true` (consuming the arm) iff a crash is armed for exactly this /// `point` and `key`. pub(crate) fn should_crash_at(point: CrashPoint, key: &str) -> bool { diff --git a/crates/ecstore/src/disk/disk_store.rs b/crates/ecstore/src/disk/disk_store.rs index a3a73464d..147c261ae 100644 --- a/crates/ecstore/src/disk/disk_store.rs +++ b/crates/ecstore/src/disk/disk_store.rs @@ -2327,6 +2327,51 @@ impl LocalDiskWrapper { .await } + pub(crate) async fn try_publish_prepared_delete_local( + &self, + volume: &str, + object: &str, + transaction: uuid::Uuid, + namespace_owner: Option>, + ) -> Result> { + self.track_disk_health_mutation( + "settle_delete_transaction", + DiskMetricMutation::Delete, + || { + Box::pin(async { + self.disk + .try_publish_prepared_delete_local(volume, object, transaction, namespace_owner) + .await + }) + }, + crate::disk::get_delete_recovery_timeout()?, + ) + .await + } + + pub(crate) async fn try_finish_and_finalize_prepared_delete_local( + &self, + volume: &str, + object: &str, + transaction: uuid::Uuid, + journal: &crate::disk::DeleteRecoveryJournal, + namespace_owner: Option>, + ) -> Result { + self.track_disk_health_mutation( + "settle_delete_transaction", + DiskMetricMutation::Delete, + || { + Box::pin(async { + self.disk + .try_finish_and_finalize_prepared_delete_local(volume, object, transaction, journal, namespace_owner) + .await + }) + }, + crate::disk::get_delete_recovery_timeout()?, + ) + .await + } + /// Track disk health for an operation. /// This method should wrap disk operations to ensure health checking. pub async fn track_disk_health(&self, operation: F, timeout_duration: Duration) -> Result diff --git a/crates/ecstore/src/disk/fs.rs b/crates/ecstore/src/disk/fs.rs index f22e4463a..8bd277727 100644 --- a/crates/ecstore/src/disk/fs.rs +++ b/crates/ecstore/src/disk/fs.rs @@ -304,6 +304,7 @@ pub async fn cached_access(path: impl AsRef) -> io::Result<()> { } pub fn invalidate_bucket_cache(path: impl AsRef) { + crate::bucket::metadata_sys::bump_local_meta_mutation_gen(); BUCKET_EXISTENCE_CACHE.invalidate(path.as_ref()); } diff --git a/crates/ecstore/src/disk/local.rs b/crates/ecstore/src/disk/local.rs index a034cd1a9..527041320 100644 --- a/crates/ecstore/src/disk/local.rs +++ b/crates/ecstore/src/disk/local.rs @@ -74,8 +74,8 @@ use crate::disk::{ error_conv::{to_access_error, to_file_error, to_unformatted_disk_error, to_volume_error}, format::FormatV3, fs::{ - O_APPEND, O_CREATE, O_RDONLY, O_TRUNC, O_WRONLY, access, cached_access, invalidate_bucket_cache, lstat, lstat_std, - remove, remove_all_std, remove_std, rename, + O_APPEND, O_CREATE, O_RDONLY, O_WRONLY, access, cached_access, invalidate_bucket_cache, lstat, lstat_std, remove, + remove_all_std, remove_std, rename, }, is_quota_mutation_fence_path, os, os::{check_path_length, is_dir_not_empty_error, is_empty_dir, is_root_disk, rename_all, rename_all_ignore_missing_source}, @@ -2985,6 +2985,51 @@ fn has_inline_commit_test_failpoints(dst_path: &str) -> bool { == Some(dst_path) } +#[cfg(all(test, target_os = "linux"))] +pub(super) fn has_rename_data_test_failpoints(dst_path: &str) -> bool { + has_inline_commit_test_failpoints(dst_path) + || INLINE_PREPARATION_BEFORE_BACKUP + .lock() + .expect("test preparation hook lock should not be poisoned") + .contains_key(dst_path) + || !RENAME_DATA_AFTER_FIRST_PUBLICATION + .lock() + .expect("test publication hook lock should not be poisoned") + .is_empty() + || RENAME_DATA_FAIL_BEFORE_OLD_METADATA_BACKUP + .lock() + .expect("test failpoint lock should not be poisoned") + .as_deref() + == Some(dst_path) + || RENAME_DATA_FAIL_AFTER_METADATA_COMMIT + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .iter() + .any(|target| target == dst_path) + || RENAME_DATA_FAIL_COMMIT_RENAME + .lock() + .expect("test failpoint lock should not be poisoned") + .as_deref() + == Some(dst_path) + || os::has_dir_fsync_test_failures() + || os::file_sync_probe::is_active() +} + +#[cfg(all(not(test), feature = "test-util", target_os = "linux"))] +pub(super) fn has_rename_data_test_failpoints(dst_path: &str) -> bool { + RENAME_DATA_FAIL_AFTER_METADATA_COMMIT + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .iter() + .any(|target| target == dst_path) + || os::has_dir_fsync_test_failures() +} + +#[cfg(any(not(any(test, feature = "test-util")), not(target_os = "linux")))] +pub(super) fn has_rename_data_test_failpoints(_dst_path: &str) -> bool { + false +} + #[cfg(test)] fn run_inline_preparation_before_backup(dst_path: &str) { let hook = INLINE_PREPARATION_BEFORE_BACKUP @@ -3056,6 +3101,43 @@ fn should_fail_after_delete_data_staged(path: &str) -> bool { } } +#[cfg(test)] +pub(crate) fn has_delete_version_test_failpoints() -> bool { + #[cfg(not(windows))] + if os::prepared_publication_test_hooks::has_any_hooks() { + return true; + } + #[cfg(feature = "e2e-test-hooks")] + if std::env::var_os("RUSTFS_DELETE_RECOVERY_BARRIER_DIR").is_some() { + return true; + } + !OWNED_FILE_WRITE_BEFORE_OPEN + .lock() + .expect("test file write hook lock should not be poisoned") + .is_empty() + || !DELETE_VERSION_FAIL_AFTER_DATA_STAGED + .lock() + .expect("test failpoint lock should not be poisoned") + .is_empty() + || !DELETE_VERSION_FAIL_AFTER_COMMIT + .lock() + .expect("test failpoint lock should not be poisoned") + .is_empty() +} + +#[cfg(not(test))] +pub(crate) fn has_delete_version_test_failpoints() -> bool { + #[cfg(all(feature = "test-util", not(windows)))] + if os::prepared_publication_test_hooks::has_any_hooks() { + return true; + } + #[cfg(feature = "e2e-test-hooks")] + if std::env::var_os("RUSTFS_DELETE_RECOVERY_BARRIER_DIR").is_some() { + return true; + } + false +} + #[cfg(test)] fn should_fail_after_delete_commit(root: &Path, path: &str) -> bool { let mut targets = DELETE_VERSION_FAIL_AFTER_COMMIT @@ -3590,6 +3672,283 @@ impl AsyncRead for FileCacheReclaimReader { } } +#[cfg(target_os = "linux")] +fn reclaim_std_file_cache(file: &std::fs::File, reclaim_len: usize, reclaim_on_shutdown: bool) { + use core::num::NonZeroU64; + use rustix::fs::{Advice, fadvise}; + + if !reclaim_on_shutdown || reclaim_len == 0 { + return; + } + let started = std::time::Instant::now(); + if let Some(len) = u64::try_from(reclaim_len).ok().and_then(NonZeroU64::new) { + if let Err(err) = fadvise(file, 0, Some(len), Advice::DontNeed) { + record_file_cache_reclaim_error("write"); + debug!(error = ?err, reclaim_len, "failed to reclaim file cache after write"); + } else { + record_file_cache_reclaim_success("write", reclaim_len, started); + } + } +} + +#[cfg(not(target_os = "linux"))] +fn reclaim_std_file_cache(_file: &std::fs::File, _reclaim_len: usize, _reclaim_on_shutdown: bool) {} + +static PRESYNCED_TMP_DATA_DIRS: std::sync::LazyLock>> = + std::sync::LazyLock::new(|| ParkingLotMutex::new(HashSet::new())); + +fn mark_presynced_tmp_data_dir(dir: PathBuf) { + let mut set = PRESYNCED_TMP_DATA_DIRS.lock(); + if set.len() >= 4096 { + set.clear(); + } + set.insert(dir); +} + +fn unmark_presynced_tmp_data_dir(dir: &Path) { + PRESYNCED_TMP_DATA_DIRS.lock().remove(dir); +} + +#[cfg(target_os = "linux")] +pub(in crate::disk) fn take_presynced_tmp_data_dir(dir: &Path) -> bool { + PRESYNCED_TMP_DATA_DIRS.lock().remove(dir) +} + +fn open_lazy_local_file_std(file_path: &Path, volume_dir: &Path, _reclaim_on_shutdown: bool) -> std::io::Result { + if let Some(parent) = file_path.parent() + && (volume_dir.as_os_str().is_empty() || !volume_dir.starts_with(parent)) + && let Err(err) = super::fs::create_dir_all_std(parent) + && err.kind() != std::io::ErrorKind::AlreadyExists + { + return Err(to_file_error(err)); + } + let file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(file_path) + .map_err(to_file_error)?; + #[cfg(target_os = "macos")] + if _reclaim_on_shutdown { + let _ = set_std_fd_nocache(&file); + } + Ok(file) +} + +struct LazyLocalFileWriter { + file_path: PathBuf, + volume_dir: PathBuf, + reclaim_len: usize, + reclaim_on_shutdown: bool, + sync_on_close: bool, + bytes_written: usize, + state: LazyLocalFileWriterState, +} + +enum LazyLocalFileWriterState { + Unopened, + Writing(tokio::task::JoinHandle, usize)>>), + Open(std::fs::File), + Closing(tokio::task::JoinHandle>), + Closed, +} + +impl LazyLocalFileWriter { + fn new(file_path: PathBuf, volume_dir: PathBuf, reclaim_len: usize, reclaim_on_shutdown: bool, sync_on_close: bool) -> Self { + Self { + file_path, + volume_dir, + reclaim_len, + reclaim_on_shutdown, + sync_on_close, + bytes_written: 0, + state: LazyLocalFileWriterState::Unopened, + } + } + + fn start_write_payload(&mut self, payload: Vec) { + let len = payload.len(); + let close_after = self.reclaim_len > 0 && self.bytes_written.saturating_add(len) >= self.reclaim_len; + let reclaim_len = self.bytes_written.saturating_add(len).max(self.reclaim_len); + let reclaim_on_shutdown = self.reclaim_on_shutdown; + let sync_on_close = self.sync_on_close; + match std::mem::replace(&mut self.state, LazyLocalFileWriterState::Closed) { + LazyLocalFileWriterState::Unopened => { + let file_path = self.file_path.clone(); + let volume_dir = self.volume_dir.clone(); + self.state = LazyLocalFileWriterState::Writing(tokio::task::spawn_blocking(move || { + let mut file = open_lazy_local_file_std(&file_path, &volume_dir, reclaim_on_shutdown)?; + std::io::Write::write_all(&mut file, &payload).map_err(to_file_error)?; + if close_after { + if sync_on_close { + file.sync_data().map_err(to_file_error)?; + } + reclaim_std_file_cache(&file, reclaim_len, reclaim_on_shutdown); + drop(file); + if sync_on_close && let Some(parent) = file_path.parent() { + mark_presynced_tmp_data_dir(parent.to_path_buf()); + } + Ok((None, len)) + } else { + Ok((Some(file), len)) + } + })); + } + LazyLocalFileWriterState::Open(mut file) => { + let file_path = self.file_path.clone(); + self.state = LazyLocalFileWriterState::Writing(tokio::task::spawn_blocking(move || { + std::io::Write::write_all(&mut file, &payload).map_err(to_file_error)?; + if close_after { + if sync_on_close { + file.sync_data().map_err(to_file_error)?; + } + reclaim_std_file_cache(&file, reclaim_len, reclaim_on_shutdown); + drop(file); + if sync_on_close && let Some(parent) = file_path.parent() { + mark_presynced_tmp_data_dir(parent.to_path_buf()); + } + Ok((None, len)) + } else { + Ok((Some(file), len)) + } + })); + } + other => self.state = other, + } + } + + fn poll_in_flight_write(&mut self, cx: &mut std::task::Context<'_>) -> std::task::Poll> { + let LazyLocalFileWriterState::Writing(fut) = &mut self.state else { + return std::task::Poll::Ready(Ok(0)); + }; + match std::pin::Pin::new(fut).poll(cx) { + std::task::Poll::Pending => std::task::Poll::Pending, + std::task::Poll::Ready(Err(join_err)) => { + self.state = LazyLocalFileWriterState::Closed; + std::task::Poll::Ready(Err(std::io::Error::other(join_err))) + } + std::task::Poll::Ready(Ok(Err(err))) => { + self.state = LazyLocalFileWriterState::Closed; + std::task::Poll::Ready(Err(err)) + } + std::task::Poll::Ready(Ok(Ok((maybe_file, written)))) => { + self.bytes_written = self.bytes_written.saturating_add(written); + self.state = maybe_file.map_or(LazyLocalFileWriterState::Closed, LazyLocalFileWriterState::Open); + std::task::Poll::Ready(Ok(written)) + } + } + } +} + +impl AsyncWrite for LazyLocalFileWriter { + fn poll_write( + mut self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + buf: &[u8], + ) -> std::task::Poll> { + if matches!(self.state, LazyLocalFileWriterState::Writing(_)) { + return self.poll_in_flight_write(cx); + } + if buf.is_empty() { + return std::task::Poll::Ready(Ok(0)); + } + if matches!(self.state, LazyLocalFileWriterState::Closed | LazyLocalFileWriterState::Closing(_)) { + return std::task::Poll::Ready(Err(std::io::Error::new( + std::io::ErrorKind::BrokenPipe, + "local file writer is already closed", + ))); + } + self.start_write_payload(buf.to_vec()); + self.poll_in_flight_write(cx) + } + + fn poll_write_vectored( + mut self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + bufs: &[std::io::IoSlice<'_>], + ) -> std::task::Poll> { + if matches!(self.state, LazyLocalFileWriterState::Writing(_)) { + return self.poll_in_flight_write(cx); + } + let total_len: usize = bufs.iter().map(|b| b.len()).sum(); + if total_len == 0 { + return std::task::Poll::Ready(Ok(0)); + } + if matches!(self.state, LazyLocalFileWriterState::Closed | LazyLocalFileWriterState::Closing(_)) { + return std::task::Poll::Ready(Err(std::io::Error::new( + std::io::ErrorKind::BrokenPipe, + "local file writer is already closed", + ))); + } + let mut payload = Vec::with_capacity(total_len); + for slice in bufs { + payload.extend_from_slice(slice); + } + self.start_write_payload(payload); + self.poll_in_flight_write(cx) + } + + fn is_write_vectored(&self) -> bool { + true + } + + fn poll_flush(mut self: std::pin::Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> std::task::Poll> { + if matches!(self.state, LazyLocalFileWriterState::Writing(_)) { + std::task::ready!(self.poll_in_flight_write(cx))?; + } + std::task::Poll::Ready(Ok(())) + } + + fn poll_shutdown( + mut self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + if matches!(self.state, LazyLocalFileWriterState::Writing(_)) { + std::task::ready!(self.poll_in_flight_write(cx))?; + } + if !matches!(self.state, LazyLocalFileWriterState::Closing(_)) { + match std::mem::replace(&mut self.state, LazyLocalFileWriterState::Closed) { + LazyLocalFileWriterState::Closed => return std::task::Poll::Ready(Ok(())), + LazyLocalFileWriterState::Unopened => { + let file_path = self.file_path.clone(); + let volume_dir = self.volume_dir.clone(); + let reclaim_on_shutdown = self.reclaim_on_shutdown; + self.state = LazyLocalFileWriterState::Closing(tokio::task::spawn_blocking(move || { + open_lazy_local_file_std(&file_path, &volume_dir, reclaim_on_shutdown).map(drop) + })); + } + LazyLocalFileWriterState::Open(file) => { + let file_path = self.file_path.clone(); + let reclaim_len = self.bytes_written.max(self.reclaim_len); + let reclaim_on_shutdown = self.reclaim_on_shutdown; + let sync_on_close = self.sync_on_close; + self.state = LazyLocalFileWriterState::Closing(tokio::task::spawn_blocking(move || { + if sync_on_close { + file.sync_data().map_err(to_file_error)?; + } + reclaim_std_file_cache(&file, reclaim_len, reclaim_on_shutdown); + drop(file); + if sync_on_close && let Some(parent) = file_path.parent() { + mark_presynced_tmp_data_dir(parent.to_path_buf()); + } + Ok(()) + })); + } + other => { + self.state = other; + } + } + } + let LazyLocalFileWriterState::Closing(fut) = &mut self.state else { + return std::task::Poll::Ready(Ok(())); + }; + let res = std::task::ready!(std::pin::Pin::new(fut).poll(cx)); + self.state = LazyLocalFileWriterState::Closed; + std::task::Poll::Ready(res.unwrap_or_else(|join_err| Err(std::io::Error::other(join_err)))) + } +} + +#[cfg(test)] impl FileCacheReclaimWriter { fn new(inner: File, reclaim_len: usize, reclaim_on_shutdown: bool) -> Self { #[cfg(target_os = "macos")] @@ -3630,6 +3989,7 @@ impl FileCacheReclaimWriter { } } +#[cfg(test)] impl AsyncWrite for FileCacheReclaimWriter { fn poll_write( mut self: std::pin::Pin<&mut Self>, @@ -3727,6 +4087,7 @@ fn should_reclaim_file_cache_after_read(length: usize) -> bool { #[derive(Clone, Copy, Debug)] pub(crate) enum WriteMode { Truncate { size_hint: i64 }, + TruncateSync { size_hint: i64 }, Append, } @@ -4494,13 +4855,16 @@ impl LocalIoBackend for StdBackend { async fn open_write(&self, volume: &str, path: &str, mode: WriteMode) -> Result { self.invalidate_cached_fd(volume, path).await; match mode { - WriteMode::Truncate { size_hint } => { + WriteMode::Truncate { size_hint } | WriteMode::TruncateSync { size_hint } => { + let sync_on_close = matches!(mode, WriteMode::TruncateSync { .. }); let volume_dir = local_disk_bucket_path(self.io_root(), volume)?; let file_path = local_disk_object_path(self.io_root(), volume, path)?; check_path_length(file_path.to_string_lossy().as_ref())?; - if let Some(parent) = file_path.parent() { - os::make_dir_all(parent, &volume_dir).await?; + check_path_length(parent.to_string_lossy().as_ref())?; + if volume == RUSTFS_META_TMP_BUCKET { + unmark_presynced_tmp_data_dir(parent); + } } // O_DIRECT streaming write (Linux, opt-in): shard bytes stream @@ -4511,6 +4875,9 @@ impl LocalIoBackend for StdBackend { // EINVAL (which would masquerade as a missing shard). #[cfg(target_os = "linux")] if is_direct_io_write_enabled() && self.direct_io_write.supported.load(Ordering::Relaxed) { + if let Some(parent) = file_path.parent() { + os::make_dir_all(parent, &volume_dir).await?; + } let write_state = self.direct_io_write.clone(); let direct_path = file_path.clone(); let direct = tokio::task::spawn_blocking(move || open_direct_writer(&direct_path, &write_state)) @@ -4521,14 +4888,14 @@ impl LocalIoBackend for StdBackend { } } - // O_TRUNC: if a file already exists at this path, stale trailing bytes past - // the new content would otherwise survive and mismatch the metadata size. - let f = super::fs::open_file(&file_path, O_CREATE | O_WRONLY | O_TRUNC) - .await - .map_err(to_file_error)?; let reclaim_on_shutdown = should_reclaim_file_cache_after_write(size_hint); - - Ok(Box::new(FileCacheReclaimWriter::new(f, size_hint.max(0) as usize, reclaim_on_shutdown))) + Ok(Box::new(LazyLocalFileWriter::new( + file_path, + volume_dir, + size_hint.max(0) as usize, + reclaim_on_shutdown, + sync_on_close, + ))) } WriteMode::Append => { let volume_dir = local_disk_bucket_path(self.io_root(), volume)?; @@ -7077,6 +7444,9 @@ impl LocalDisk { fi: FileInfo, namespace_owner: Option>, ) -> Result<()> { + if volume == RUSTFS_META_BUCKET { + crate::bucket::metadata_sys::bump_local_meta_mutation_gen(); + } if volume == RUSTFS_META_MULTIPART_BUCKET { crate::disk::multipart_source::native_source_protocol(&fi)?; } @@ -7323,6 +7693,9 @@ impl LocalDisk { mut fi: FileInfo, mutation: DeleteVersionMutation, ) -> Result<()> { + if volume == RUSTFS_META_BUCKET { + crate::bucket::metadata_sys::bump_local_meta_mutation_gen(); + } pool_meta_native::reject_noncanonical_object(&self.root, volume, path)?; let DeleteVersionMutation { force_del_marker, @@ -7394,6 +7767,13 @@ impl LocalDisk { } let rollback_dir = opts.old_data_dir; + if let Some(rollback_dir) = rollback_dir + && let Some(()) = self + .try_fused_recoverable_delete_version(volume, path, &fi, &opts, rollback_dir, namespace_owner.clone()) + .await? + { + return Ok(()); + } let buf = match self.read_all_data(volume, &volume_dir, &xl_path).await { Ok(res) => res, Err(err) => { @@ -7919,6 +8299,9 @@ impl LocalDisk { opt: DeleteOptions, namespace_owner: Option>, ) -> Result<()> { + if volume == RUSTFS_META_BUCKET { + crate::bucket::metadata_sys::bump_local_meta_mutation_gen(); + } multipart_staging::reject_native_control_write(&self.root, self.io_root(), volume, path)?; let mut source_owner = self.acquire_multipart_delete_owner(volume, path, namespace_owner).await?; let namespace_owner = source_owner.namespace_owner(); @@ -9536,6 +9919,9 @@ impl LocalDisk { // write_all_private with check_path_length #[tracing::instrument(level = "trace", skip_all)] async fn write_all_private(&self, volume: &str, path: &str, buf: Bytes, sync: SyncMode, skip_parent: &Path) -> Result<()> { + if volume == RUSTFS_META_BUCKET { + crate::bucket::metadata_sys::bump_local_meta_mutation_gen(); + } let file_path = self.io_get_object_path(volume, path)?; check_path_length(file_path.to_string_lossy().as_ref())?; @@ -11476,20 +11862,32 @@ pub(crate) async fn batch_shard_pread(requests: Vec<(std::path::PathBuf, usize, return Err(DiskError::FileCorrupt); } - let mut buf = diagnostic_stage!(Allocation, _index, || Ok::<_, DiskError>(vec![0u8; length]))?; + let mut buf = diagnostic_stage!(Allocation, _index, || Ok::<_, DiskError>(Vec::with_capacity(length)))?; if length > 0 { diagnostic_stage!(Read, _index, || measure_local_batch_stage( metrics.as_deref(), LocalBatchReadStage::Read, || { - while total < length { - let nbytes = file - .read_at(&mut buf[total..], u64::try_from(offset + total).unwrap_or(u64::MAX)) + if offset == 0 { + use std::io::Read as _; + total = (&file) + .take(u64::try_from(length).unwrap_or(u64::MAX)) + .read_to_end(&mut buf) .map_err(DiskError::from)?; - if nbytes == 0 { + if total < length { return Err(DiskError::FileCorrupt); } - total += nbytes; + } else { + buf.resize(length, 0); + while total < length { + let nbytes = file + .read_at(&mut buf[total..], u64::try_from(offset + total).unwrap_or(u64::MAX)) + .map_err(DiskError::from)?; + if nbytes == 0 { + return Err(DiskError::FileCorrupt); + } + total += nbytes; + } } Ok::<_, DiskError>(()) } @@ -12741,9 +13139,23 @@ impl DiskAPI for LocalDisk { } } - self.io_backend - .open_write(volume, path, WriteMode::Truncate { size_hint: _file_size }) - .await + #[cfg(test)] + let probe_active = os::file_sync_probe::is_active(); + #[cfg(not(test))] + let probe_active = false; + let sync_on_close = cfg!(target_os = "linux") + && volume == RUSTFS_META_TMP_BUCKET + && !origvolume.is_empty() + && path.ends_with("/part.1") + && effective_durability(origvolume).syncs_data_shards() + && !os::file_fdatasync_group_commit_enabled() + && !probe_active; + let mode = if sync_on_close { + WriteMode::TruncateSync { size_hint: _file_size } + } else { + WriteMode::Truncate { size_hint: _file_size } + }; + self.io_backend.open_write(volume, path, mode).await } async fn rename_file_durable(&self, src_volume: &str, src_path: &str, dst_volume: &str, dst_path: &str) -> Result<()> { @@ -13308,16 +13720,70 @@ impl DiskAPI for LocalDisk { check_path_length(file_path.to_string_lossy().as_ref())?; - let buf = self - .read_all(volume, format!("{}/{}", path, STORAGE_FORMAT_FILE).as_str()) - .await - .map_err(|e| { - if e == DiskError::FileNotFound && fi.version_id.is_some() { - DiskError::FileVersionNotFound - } else { - e + let xl_rel_path = format!("{path}/{STORAGE_FORMAT_FILE}"); + if self.multipart_source_admission(volume, &xl_rel_path, true, false)?.is_none() + && !crash_inject::is_armed(CrashPoint::MetaWriteAfterTmpBeforeRename, &xl_rel_path) + && !has_delete_version_test_failpoints() + && !has_rename_data_test_failpoints(&xl_rel_path) + { + multipart_staging::reject_reserved_anchor(&xl_rel_path)?; + let volume_dir = self.io_get_bucket_path(volume)?; + let xl_file_path = self.io_get_object_path(volume, &xl_rel_path)?; + check_path_length(xl_file_path.to_string_lossy().as_ref())?; + let tmp_file_path = self.io_get_object_path(RUSTFS_META_TMP_BUCKET, Uuid::new_v4().to_string().as_str())?; + let sync = !opts.no_persistence && effective_durability(volume).syncs_commit_metadata(); + let replace_user_metadata = opts.replace_user_metadata; + let has_version_id = fi.version_id.is_some(); + let publication_root = self.publication_root.clone(); + let lease = os::acquire_namespace_mutation_lease_with_owner(&xl_file_path, namespace_owner).await; + return os::run_blocking_namespace_operation_with_sync(lease, sync, move || -> std::io::Result> { + let buf = match std::fs::read(&xl_file_path) { + Ok(buf) => buf, + Err(err) if err.kind() == ErrorKind::NotFound && has_version_id => { + return Ok(Err(DiskError::FileVersionNotFound)); + } + Err(err) => return Err(err), + }; + if !FileMeta::is_xl2_v1_format(&buf) { + return Ok(Err(DiskError::FileVersionNotFound)); } - })?; + let wbuf = match FileMeta::load(&buf).and_then(|mut xl_meta| { + xl_meta.update_object_version_with_opts(fi, replace_user_metadata)?; + xl_meta.marshal_msg() + }) { + Ok(wbuf) => wbuf, + Err(err) => return Ok(Err(err.into())), + }; + if let Some(parent) = tmp_file_path.parent() { + std::fs::create_dir_all(parent)?; + } + let mut tmp_file = std::fs::File::create(&tmp_file_path)?; + std::io::Write::write_all(&mut tmp_file, &wbuf)?; + if sync && !wbuf.is_empty() { + tmp_file.sync_data()?; + } + drop(tmp_file); + os::rename_all_with_precondition_and_parent_sync_std( + &tmp_file_path, + &xl_file_path, + &volume_dir, + &publication_root, + None, + sync, + )?; + Ok(Ok(())) + }) + .await + .map_err(to_file_error)?; + } + + let buf = self.read_all(volume, xl_rel_path.as_str()).await.map_err(|e| { + if e == DiskError::FileNotFound && fi.version_id.is_some() { + DiskError::FileVersionNotFound + } else { + e + } + })?; if !FileMeta::is_xl2_v1_format(buf.as_ref()) { return Err(DiskError::FileVersionNotFound); @@ -13330,13 +13796,7 @@ impl DiskAPI for LocalDisk { let wbuf = xl_meta.marshal_msg()?; return self - .write_all_meta_with_namespace_owner( - volume, - format!("{path}/{STORAGE_FORMAT_FILE}").as_str(), - &wbuf, - !opts.no_persistence, - namespace_owner, - ) + .write_all_meta_with_namespace_owner(volume, xl_rel_path.as_str(), &wbuf, !opts.no_persistence, namespace_owner) .await; } @@ -17417,6 +17877,54 @@ mod test { ); } + #[cfg(target_os = "linux")] + #[tokio::test] + #[serial_test::serial(dst_dir_fsync_group_commit, file_fdatasync_group_commit)] + #[allow(clippy::await_holding_lock)] + async fn rename_data_inline_new_version_fsync_failure_preserves_previous_metadata() { + let _group_commit = os::set_dst_dir_fsync_group_commit_for_test(false); + let _file_group_commit = os::set_file_fdatasync_group_commit_for_test(false); + let _mode = durability_mode_override::set(DurabilityMode::Strict); + let dir = tempfile::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 = "inline-version-fsync-failure"; + let object = "dir/object"; + ensure_test_volume(&disk, bucket).await; + ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await; + + let old_version = Uuid::new_v4(); + let old_fi = test_file_info(object, old_version, None, Some(Bytes::from_static(b"previous-inline-version"))); + disk.rename_data(RUSTFS_META_TMP_BUCKET, "old-inline-stage", old_fi, bucket, object) + .await + .expect("previous inline version should commit"); + let dst_meta = disk + .get_object_path(bucket, &format!("{object}/{STORAGE_FORMAT_FILE}")) + .expect("metadata path should resolve"); + let previous = fs::read(&dst_meta).await.expect("previous metadata should be readable"); + let destination_dir = dst_meta.parent().expect("metadata should have a parent").to_path_buf(); + // Arm the failure after the fused-path selection so the test exercises + // its normal admission path instead of the failpoint fallback. + set_inline_before_file_sync_admission(object, move || { + os::fsync_dir_recorder::set_failure(&destination_dir, io::ErrorKind::PermissionDenied); + }); + + let new_version = Uuid::new_v4(); + let new_fi = test_file_info(object, new_version, None, Some(Bytes::from_static(b"incoming-inline-version"))); + let err = disk + .rename_data(RUSTFS_META_TMP_BUCKET, "new-inline-stage", new_fi, bucket, object) + .await + .expect_err("directory fsync failure must reject the new version"); + assert!(matches!(err, DiskError::Io(ref err) if err.kind() == io::ErrorKind::PermissionDenied)); + let restored = fs::read(&dst_meta) + .await + .expect("previous metadata must survive the failed commit"); + assert_eq!(restored, previous, "rollback must restore all previously committed inline versions"); + let metadata = FileMeta::load(&restored).expect("restored metadata must be valid"); + assert!(metadata.find_version(Some(old_version)).is_ok()); + assert!(metadata.find_version(Some(new_version)).is_err()); + } + // Seed a first PUT of `object` (no prior version) through the non-inline // rename_data path and return (disk, tempdir). The object dir and any prefix // dirs are created during the commit. @@ -19643,6 +20151,116 @@ mod test { ); } + #[cfg(target_os = "linux")] + #[tokio::test] + #[serial_test::serial(dst_dir_fsync_group_commit, file_fdatasync_group_commit, file_sync_probe)] + #[allow(clippy::await_holding_lock)] + async fn test_rename_data_unknown_destination_defers_shard_sync() { + let _group_commit = os::set_dst_dir_fsync_group_commit_for_test(false); + let _file_group_commit = os::set_file_fdatasync_group_commit_for_test(false); + let _mode = durability_mode_override::set(DurabilityMode::Strict); + for destination_mode in [DurabilityMode::None, DurabilityMode::Strict] { + let bucket = "unknown-destination-shard-bucket"; + let _bucket_mode = BucketOverrideGuard::set(bucket, destination_mode); + let dir = tempfile::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 object = "dir/object"; + let tmp_object = "tmp-unknown-destination"; + let data_dir = Uuid::new_v4(); + ensure_test_volume(&disk, bucket).await; + ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await; + + let shard_path = format!("{tmp_object}/{data_dir}/part.1"); + let mut writer = disk + .create_file("", RUSTFS_META_TMP_BUCKET, &shard_path, 8) + .await + .expect("open staged shard writer without a destination"); + writer.write_all(b"new-data").await.expect("write staged shard"); + writer.shutdown().await.expect("close staged shard"); + let staged_dir = disk + .get_object_path_for_io(RUSTFS_META_TMP_BUCKET, &format!("{tmp_object}/{data_dir}")) + .expect("staged shard directory should resolve"); + assert!( + !PRESYNCED_TMP_DATA_DIRS.lock().contains(&staged_dir), + "an unknown destination must defer payload sync until commit" + ); + + let mut fi = test_file_info(object, Uuid::new_v4(), Some(data_dir), None); + fi.size = 8; + fi.parts[0].size = 8; + fi.parts[0].actual_size = 8; + disk.rename_data(RUSTFS_META_TMP_BUCKET, tmp_object, fi, bucket, object) + .await + .expect("rename_data should apply the destination policy"); + assert_eq!( + os::fsync_dir_recorder::was_fsynced(&staged_dir), + destination_mode.syncs_data_shards(), + "the commit must apply the destination durability override" + ); + let shard = disk + .get_object_path(bucket, &format!("{object}/{data_dir}/part.1")) + .expect("committed shard should resolve"); + assert_eq!(fs::read(shard).await.expect("committed shard should be readable"), b"new-data"); + } + } + + #[cfg(target_os = "linux")] + #[tokio::test] + #[serial_test::serial(dst_dir_fsync_group_commit, file_fdatasync_group_commit, file_sync_probe)] + #[allow(clippy::await_holding_lock)] + async fn test_rename_data_presynced_shard_keeps_directory_sync() { + let _group_commit = os::set_dst_dir_fsync_group_commit_for_test(false); + let _file_group_commit = os::set_file_fdatasync_group_commit_for_test(false); + for mode in [DurabilityMode::Strict, DurabilityMode::Relaxed] { + let _mode = durability_mode_override::set(mode); + let dir = tempfile::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 = "presynced-shard-bucket"; + let object = "dir/object"; + let tmp_object = "tmp-presynced-write"; + let data_dir = Uuid::new_v4(); + ensure_test_volume(&disk, bucket).await; + ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await; + + let shard_path = format!("{tmp_object}/{data_dir}/part.1"); + let mut writer = disk + .create_file(bucket, RUSTFS_META_TMP_BUCKET, &shard_path, 8) + .await + .expect("open staged shard writer"); + writer.write_all(b"new-data").await.expect("write staged shard"); + writer.shutdown().await.expect("close staged shard"); + let staged_dir = disk + .get_object_path_for_io(RUSTFS_META_TMP_BUCKET, &format!("{tmp_object}/{data_dir}")) + .expect("staged shard directory should resolve"); + assert!(PRESYNCED_TMP_DATA_DIRS.lock().contains(&staged_dir), "the writer must pre-sync the shard"); + assert!(!os::fsync_dir_recorder::was_fsynced(&staged_dir)); + + let mut fi = test_file_info(object, Uuid::new_v4(), Some(data_dir), None); + fi.size = 8; + fi.parts[0].size = 8; + fi.parts[0].actual_size = 8; + disk.rename_data(RUSTFS_META_TMP_BUCKET, tmp_object, fi, bucket, object) + .await + .expect("rename_data should commit"); + assert!( + !PRESYNCED_TMP_DATA_DIRS.lock().contains(&staged_dir), + "the fused commit must consume the pre-sync marker" + ); + assert!( + os::fsync_dir_recorder::was_fsynced(&staged_dir), + "pre-syncing the payload must not skip the shard directory barrier" + ); + let shard = disk + .get_object_path(bucket, &format!("{object}/{data_dir}/part.1")) + .expect("committed shard should resolve"); + assert_eq!(fs::read(shard).await.expect("committed shard should be readable"), b"new-data"); + } + } + #[tokio::test] #[allow(clippy::await_holding_lock)] async fn test_rename_data_legacy_off_skips_all_fsyncs() { @@ -19663,17 +20281,20 @@ mod test { ensure_test_volume(&disk, bucket).await; ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await; - let tmp_data_dir = dir - .path() - .join(RUSTFS_META_TMP_BUCKET) - .join(tmp_object) - .join(new_data_dir.to_string()); - fs::create_dir_all(&tmp_data_dir) + let shard_path = format!("{tmp_object}/{new_data_dir}/part.1"); + let mut writer = disk + .create_file("", RUSTFS_META_TMP_BUCKET, &shard_path, 8) .await - .expect("new tmp data dir should be created"); - fs::write(tmp_data_dir.join("part.1"), b"new-data") - .await - .expect("new tmp data should be written"); + .expect("open staged shard writer"); + writer.write_all(b"new-data").await.expect("write staged shard"); + writer.shutdown().await.expect("close staged shard"); + let resolved_tmp_data_dir = disk + .get_object_path(RUSTFS_META_TMP_BUCKET, &format!("{tmp_object}/{new_data_dir}")) + .expect("tmp data dir path should resolve"); + assert!( + !os::fsync_dir_recorder::was_fsynced(&resolved_tmp_data_dir), + "the staged writer must not sync before the destination policy is known" + ); let new_fi = test_file_info(object, version_id, Some(new_data_dir), None); disk.rename_data(RUSTFS_META_TMP_BUCKET, tmp_object, new_fi, bucket, object) diff --git a/crates/ecstore/src/disk/local/commit.rs b/crates/ecstore/src/disk/local/commit.rs index 04860ec35..83565c327 100644 --- a/crates/ecstore/src/disk/local/commit.rs +++ b/crates/ecstore/src/disk/local/commit.rs @@ -329,6 +329,38 @@ pub(super) async fn lock_rename_commit_directories( result.map_err(to_file_error).map_err(DiskError::from) } +#[cfg(target_os = "linux")] +fn prepare_rename_commit_and_destination_metadata_std( + create_source_parent: bool, + source_parent: &Path, + destination_parent: &Path, + destination_metadata: &Path, + base_dir: &Path, + publication_root: &os::PublicationRoot, +) -> std::io::Result<(os::RenameCommitGuard, Option)> { + if create_source_parent { + std::fs::create_dir_all(source_parent)?; + } + let guard = match os::prepare_rename_commit_guard(source_parent, destination_parent, base_dir, publication_root) { + Ok(guard) => guard, + Err(err) => { + return Err(match std::fs::symlink_metadata(base_dir) { + Err(base_err) if base_err.kind() == ErrorKind::NotFound => base_err, + _ => err, + }); + } + }; + let data = os::read_destination_file_with_commit_guard(destination_metadata, &guard)?; + Ok((guard, data.map(Bytes::from))) +} + +#[cfg(target_os = "linux")] +enum FusedCommitAttempt { + Committed(RenameDataResp), + CommitErr(DiskError), + Fallback(os::RenameCommitGuard, Option, FileInfo), +} + async fn prepare_rename_commit_and_destination_metadata( create_source_parent: bool, source_parent: &Path, @@ -571,6 +603,9 @@ impl LocalDisk { return Err(err); } } + if dst_volume == crate::disk::RUSTFS_META_BUCKET { + crate::bucket::metadata_sys::bump_local_meta_mutation_gen(); + } if fi.is_legacy_indexed_delete_marker() { fi.erasure.index = 0; } @@ -698,6 +733,216 @@ impl LocalDisk { let dst_file_parent = dst_file_path .parent() .ok_or_else(|| DiskError::other("missing object metadata parent"))?; + + #[cfg(target_os = "linux")] + let (rename_commit_guard, has_dst_buf) = if !preserve_upload_metadata + && multipart_publication.is_none() + && pool_meta_condition.is_none() + && !fi_healing + && !os::dst_dir_fsync_group_commit_enabled() + && !os::file_fdatasync_group_commit_enabled() + && !super::has_rename_data_test_failpoints(dst_path) + && !crash_inject::is_armed(CrashPoint::RenameAfterDataRename, dst_path) + && !crash_inject::is_armed(CrashPoint::RenameAfterBackupBeforeMetaCommit, dst_path) + && !crash_inject::is_armed(CrashPoint::RenameAfterMetaCommit, dst_path) + && !super::has_delete_version_test_failpoints() + { + #[cfg(test)] + if !no_inline { + run_inline_before_file_sync_admission(dst_path); + } + let sync_data_shards = no_inline && durability.syncs_data_shards(); + let sync_commit_metadata = durability.syncs_commit_metadata(); + let file_sync_admission = if sync_data_shards || sync_commit_metadata { + Some( + os::acquire_file_sync_admission(self.file_sync_permits.clone()) + .await + .map_err(to_file_error) + .map_err(DiskError::from)?, + ) + } else { + None + }; + let src_parent_owned = src_file_parent.to_path_buf(); + let dst_parent_owned = dst_file_parent.to_path_buf(); + let src_file_path_owned = src_file_path.clone(); + let dst_file_path_owned = dst_file_path.clone(); + let dst_volume_dir_owned = dst_volume_dir.clone(); + let publication_root_owned = self.publication_root.clone(); + let has_data_dir_path_owned = has_data_dir_path.clone(); + let dst_volume_str = dst_volume.to_string(); + let dst_path_str = dst_path.to_string(); + let fi_moved = fi; + #[cfg(test)] + let root_owned = self.root.clone(); + + let fused_attempt = move |is_cancelled: &dyn Fn() -> bool| -> std::io::Result { + let (guard, has_dst_buf) = prepare_rename_commit_and_destination_metadata_std( + !no_inline, + &src_parent_owned, + &dst_parent_owned, + &dst_file_path_owned, + &dst_volume_dir_owned, + &publication_root_owned, + )?; + // An existing xl.meta can share versions or require rollback + // restoration when a post-rename directory sync fails. + if is_cancelled() || (sync_commit_metadata && has_dst_buf.is_some()) { + return Ok(FusedCommitAttempt::Fallback(guard, has_dst_buf, fi_moved)); + } + let mut xlmeta = FileMeta::new(); + let mut dst_meta_unparsable = false; + if let Some(ref dst_buf) = has_dst_buf { + if FileMeta::is_xl2_v1_format(dst_buf) + && let Ok(nmeta) = FileMeta::load(dst_buf) + { + xlmeta = nmeta; + } else { + dst_meta_unparsable = true; + } + } + let version_id = fi_moved.version_id.unwrap_or_default(); + let has_old_data_dir = xlmeta.find_unshared_data_dir_for_version(Some(version_id)); + let rollback_data_dir = has_old_data_dir.or_else(|| { + if has_dst_buf.is_some() && xlmeta.find_version(Some(version_id)).is_ok() { + Some(inline_metadata_rollback_dir(version_id, &xlmeta)) + } else { + None + } + }); + if rollback_data_dir.is_some() { + return Ok(FusedCommitAttempt::Fallback(guard, has_dst_buf, fi_moved)); + } + let (old_current_size, old_current_source_checked, old_current_source) = observe_old_current( + has_dst_buf.is_some(), + dst_meta_unparsable, + &xlmeta, + &dst_volume_str, + &dst_path_str, + version_id, + ); + xlmeta.add_version(fi_moved)?; + let version_signature = rename_data_versions_signature(&xlmeta); + let new_dst_buf = xlmeta.marshal_msg().map_err(std::io::Error::other)?; + let destination_was_absent = has_dst_buf.is_none(); + let mut skip_parent = dst_volume_dir_owned.clone(); + if has_dst_buf.is_some() { + skip_parent = dst_parent_owned.clone(); + } + + let mut prepared_metadata_source = + os::create_prepared_rename_source_with_commit_guard(&src_file_path_owned, &dst_file_path_owned, &guard)?; + prepared_metadata_source.write_all(&new_dst_buf, sync_commit_metadata)?; + + if no_inline { + if sync_data_shards && let Some((src_data_path, _)) = has_data_dir_path_owned.as_ref() { + let sync_result = if super::take_presynced_tmp_data_dir(src_data_path) { + // The payload was synced, but its part.1 directory entry + // must also be persisted before moving the data directory. + os::fsync_dir_std(src_data_path) + } else { + os::sync_dir_files_std(src_data_path) + }; + if let Err(err) = sync_result + && err.kind() != ErrorKind::NotFound + { + return Err(err); + } + } + if let Some((src_data_path, dst_data_path)) = has_data_dir_path_owned.as_ref() + && let Err(err) = os::rename_all_with_commit_guard_on_thread(src_data_path, dst_data_path, &guard) + { + return Ok(FusedCommitAttempt::CommitErr(err)); + } + #[cfg(test)] + if has_data_dir_path_owned.is_some() { + run_rename_data_after_first_publication(&root_owned, &dst_volume_str, &dst_path_str); + } + } + + if let Err(err) = os::rename_prepared_source_on_thread( + prepared_metadata_source, + &src_file_path_owned, + &dst_file_path_owned, + &skip_parent, + &publication_root_owned, + &guard, + ) { + if no_inline && let Some((src_data_path, dst_data_path)) = has_data_dir_path_owned.as_ref() { + let _ = std::fs::rename(dst_data_path, src_data_path); + } + return Ok(FusedCommitAttempt::CommitErr(err)); + } + + if sync_commit_metadata + && let Err(err) = + fsync_inline_commit_std(&dst_file_path_owned, &dst_volume_dir_owned, destination_was_absent, None, &guard) + { + if no_inline && let Some((src_data_path, dst_data_path)) = has_data_dir_path_owned.as_ref() { + let _ = std::fs::rename(dst_data_path, src_data_path); + } + return Err(err); + } + + if let Some(parent) = src_file_path_owned.parent() { + let _ = std::fs::remove_dir(parent); + } + + Ok(FusedCommitAttempt::Committed(RenameDataResp { + multipart_source: None, + old_data_dir: None, + rollback_data_dir: None, + cleanup_data_dir: None, + sign: version_signature, + old_current_size, + old_current_source_checked, + old_current_source, + })) + }; + + let attempt_res = if let Some(admission) = file_sync_admission.as_ref() { + os::run_cancellable_blocking_namespace_file_sync_operation(mutation_lease.clone(), admission, fused_attempt).await + } else { + os::run_cancellable_blocking_namespace_operation(mutation_lease.clone(), fused_attempt).await + } + .map_err(to_file_error) + .map_err(DiskError::from); + drop(file_sync_admission); + match attempt_res { + Ok(FusedCommitAttempt::Committed(resp)) => { + for part_path in &invalidate_part_paths { + self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; + } + #[cfg(unix)] + if let Some(receipt_mutation) = receipt_mutation.as_mut() { + receipt_mutation.complete(); + } + return Ok(resp); + } + Ok(FusedCommitAttempt::CommitErr(err)) | Err(err) => { + for part_path in &invalidate_part_paths { + self.io_backend.invalidate_cached_fd(dst_volume, part_path).await; + } + return Err(err); + } + Ok(FusedCommitAttempt::Fallback(guard, dst_buf, restored_fi)) => { + fi = restored_fi; + (guard, dst_buf) + } + } + } else { + prepare_rename_commit_and_destination_metadata( + !no_inline && !preserve_upload_metadata, + src_file_parent, + dst_file_parent, + &dst_file_path, + &dst_volume_dir, + &self.publication_root, + mutation_lease.clone(), + ) + .await? + }; + #[cfg(not(target_os = "linux"))] // Acquire the common trees before reading destination metadata. On // Windows this pins the object directory identity across publication. // Multipart authority already created its source tree. Recreating it diff --git a/crates/ecstore/src/disk/local/delete_recovery.rs b/crates/ecstore/src/disk/local/delete_recovery.rs index a6bca69d4..c1cb880de 100644 --- a/crates/ecstore/src/disk/local/delete_recovery.rs +++ b/crates/ecstore/src/disk/local/delete_recovery.rs @@ -2090,6 +2090,474 @@ impl LocalDisk { pub(super) async fn delete_recovery_barrier(&self, volume: &str, object: &str, transaction: Uuid, stage: &str) -> Result<()> { barrier(volume, object, transaction, stage, &self.root).await } + + pub(super) async fn try_fused_recoverable_delete_version( + &self, + volume: &str, + object: &str, + fi: &FileInfo, + opts: &DeleteOptions, + transaction: Uuid, + owner: Option>, + ) -> Result> { + if !opts.recoverable_delete + || opts.undo_write + || opts.expected_delete_marker.is_some() + || pool_meta_native::is_object(&self.root, volume, object) + || has_delete_version_test_failpoints() + { + return Ok(None); + } + self.reject_multipart_delete_recovery_mutation(volume, object)?; + self.wait_for_startup_cleanup().await; + + let (_, file_path) = self.io_get_bucket_and_object_path(volume, object)?; + check_path_length(file_path.to_string_lossy().as_ref())?; + let xl_path = path_join(&[file_path.as_path(), Path::new(STORAGE_FORMAT_FILE)]); + let destination = file_path.join(transaction.to_string()); + let staging_root = self.io_get_bucket_path(RUSTFS_META_TMP_BUCKET)?; + let sync = effective_durability(volume).syncs_commit_metadata(); + let incarnation = opts.delete_recovery_bucket_incarnation; + let lease = os::acquire_namespace_mutation_lease_with_owner(&destination, owner).await; + let fi_clone = fi.clone(); + let volume_str = volume.to_owned(); + let object_str = object.to_owned(); + + let res = os::run_blocking_namespace_operation_with_sync( + lease, + sync, + move || -> std::io::Result>>> { + let buf = match std::fs::read(&xl_path) { + Ok(buf) => buf, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Ok(None)), + Err(err) => return Err(err), + }; + let (old_dir, journal_raw) = match (|| -> Result, Vec)>> { + let mut meta = FileMeta::load(&buf)?; + let deleted_versions: Vec<_> = meta + .versions + .iter() + .map(|v| v.header.version_id.unwrap_or_default()) + .collect(); + let original_versions: Vec<_> = meta + .versions + .iter() + .map(|v| generation(v.into_fileinfo(&volume_str, &object_str, true)?)) + .collect::>()?; + let old_dir = meta.delete_version(&fi_clone)?; + if old_dir.is_some_and(|dir| dir == transaction || dir.is_nil()) { + return Err(DiskError::FileCorrupt); + } + if let Some(uuid) = old_dir { + meta.data.remove(vec![fi_clone.version_id.unwrap_or_default(), uuid])?; + let old_path = path_join(&[file_path.as_path(), Path::new(uuid.to_string().as_str())]); + check_path_length(old_path.to_string_lossy().as_ref())?; + if old_path.join(STORAGE_FORMAT_FILE).exists() { + return Ok(None); + } + } + if !meta.versions.is_empty() { + return Ok(None); + } + let mut predecessors = BTreeSet::new(); + if let Ok(entries) = std::fs::read_dir(&file_path) { + for (visited, entry) in entries.enumerate() { + if visited >= 65536 { + return Err(DiskError::DiskOngoingReq); + } + let entry = entry.map_err(to_file_error)?; + let Some(id) = entry + .file_name() + .to_str() + .and_then(|name| Uuid::parse_str(name).ok()) + .filter(|id| !id.is_nil() && *id != transaction) + else { + continue; + }; + if !entry.file_type().map_err(to_file_error)?.is_dir() { + return Err(DiskError::InvalidPath); + } + let prepared = regular_bytes(&entry.path().join(PREPARED))?; + let receipt = regular_bytes(&entry.path().join(DELETE_QUORUM_COMMITTED_FILE))?; + let mut records = Vec::new(); + if let Some(bytes) = prepared { + records.push( + serde_json::from_slice::(&bytes) + .map_err(|_| DiskError::FileCorrupt)?, + ); + } + if let Some(bytes) = receipt.filter(|bytes| !bytes.is_empty()) { + records.push( + serde_json::from_slice::(&bytes) + .map_err(|_| DiskError::FileCorrupt)? + .journal, + ); + } + for record in records { + if record.protocol_version != DELETE_RECOVERY_PROTOCOL_VERSION + || record.volume != volume_str + || record.object != object_str + || record.transaction != id + || incarnation.zip(record.bucket_incarnation).is_some_and(|(a, b)| a != b) + || record.predecessors.iter().any(|before| before.is_nil() || *before == id) + { + return Err(DiskError::FileCorrupt); + } + predecessors.insert(id); + } + } + } + let mut sparse = fi_clone; + sparse.data = None; + let journal = DeleteRecoveryJournal { + protocol_version: DELETE_RECOVERY_PROTOCOL_VERSION, + volume: volume_str, + object: object_str, + transaction, + bucket_incarnation: incarnation, + original_digest: Some(digest(&buf)), + target_digest: None, + data_dirs: old_dir.into_iter().collect(), + deleted_versions, + requests: vec![sparse], + original_versions, + predecessors: predecessors.into_iter().collect(), + }; + Ok(Some((old_dir, journal_bytes(&journal)?))) + })() { + Ok(Some(pair)) => pair, + Ok(None) => return Ok(Ok(None)), + Err(err) => return Ok(Err(err)), + }; + std::fs::create_dir_all(&staging_root)?; + let staged = tempfile::Builder::new() + .prefix("delete-preparation-") + .tempdir_in(&staging_root)?; + for (name, bytes) in [ + (STORAGE_FORMAT_FILE_BACKUP, buf.as_slice()), + (PREPARED, journal_raw.as_slice()), + ] { + let mut file = std::fs::File::create(staged.path().join(name))?; + std::io::Write::write_all(&mut file, bytes)?; + if sync && !bytes.is_empty() { + file.sync_data()?; + } + } + if sync { + os::fsync_dir_std(staged.path())?; + } + match std::fs::symlink_metadata(&destination) { + Ok(metadata) if metadata.is_dir() => match std::fs::remove_dir(&destination) { + Ok(()) => {} + Err(err) if err.kind() == ErrorKind::NotFound => {} + Err(err) if is_dir_not_empty_error(&err) => return Ok(Err(DiskError::OutdatedXLMeta)), + Err(err) => return Err(err), + }, + Ok(_) => return Ok(Err(DiskError::OutdatedXLMeta)), + Err(err) if err.kind() == ErrorKind::NotFound => {} + Err(err) => return Err(err), + } + if let Some(parent) = destination.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::rename(staged.path(), &destination)?; + if sync { + os::fsync_dir_std(&file_path)?; + } + let mut reserved_data_path = None; + if let Some(uuid) = old_dir { + let old_path = file_path.join(uuid.to_string()); + if old_path.is_dir() { + let reserved_marker = old_path.join(format!("{RESERVED_DELETE_DATA_DIR_MARKER_PREFIX}{transaction}")); + let marker = std::fs::File::create(&reserved_marker)?; + if sync { + marker.sync_all()?; + os::fsync_dir_std(&old_path)?; + } + reserved_data_path = Some((old_path, reserved_marker)); + } + } + if let Err(err) = std::fs::remove_file(&xl_path) + && err.kind() != ErrorKind::NotFound + { + if let Some((_, reserved_marker)) = &reserved_data_path { + let _ = std::fs::remove_file(reserved_marker); + } + let _ = std::fs::remove_dir_all(&destination); + return Err(err); + } + if sync { + os::fsync_dir_std(&file_path)?; + } + if let Some((old_path, reserved_marker)) = reserved_data_path { + let recovery_marker = old_path.join(format!("{RECOVERY_DELETE_DATA_DIR_MARKER_PREFIX}{transaction}")); + match std::fs::rename(&reserved_marker, &recovery_marker) { + Ok(()) if sync => os::fsync_dir_std(&old_path)?, + Ok(()) => {} + Err(err) if err.kind() == ErrorKind::NotFound => {} + Err(err) => return Err(err), + } + } + Ok(Ok(Some(old_dir))) + }, + ) + .await + .map_err(to_file_error)??; + + let Some(old_dir) = res else { + return Ok(None); + }; + if let Some(uuid) = old_dir { + self.io_backend + .invalidate_cached_fds_under(volume, &format!("{object}/{uuid}")); + } + Ok(Some(())) + } + + pub(crate) async fn try_publish_prepared_delete_local( + &self, + volume: &str, + object: &str, + transaction: Uuid, + namespace_owner: Option>, + ) -> Result> { + self.reject_multipart_delete_recovery_mutation(volume, object)?; + if !safe_path(object, false) || transaction.is_nil() { + return Err(DiskError::InvalidPath); + } + let object_path = self.get_object_path(volume, object)?; + let metadata_owner = os::acquire_metadata_mutation_lease(&object_path, namespace_owner).await; + let volume_guard = os::disk_volume_mutation_lock(&self.root, volume).read_owned().await; + let owner: Option> = Some(Arc::new((metadata_owner, volume_guard))); + + let root = self.io_get_object_path(volume, object)?; + let rollback = root.join(transaction.to_string()); + let receipt_path = rollback.join(DELETE_QUORUM_COMMITTED_FILE); + let staging_root = self.io_get_bucket_path(RUSTFS_META_TMP_BUCKET)?; + let sync = effective_durability(volume).syncs_commit_metadata(); + let lease = os::acquire_namespace_mutation_lease_with_owner(&receipt_path, owner).await; + let volume_str = volume.to_owned(); + let object_str = object.to_owned(); + + os::run_blocking_namespace_operation_with_sync( + lease, + sync, + move || -> std::io::Result>> { + let prepared = match regular_bytes(&rollback.join(PREPARED)) { + Ok(Some(bytes)) => bytes, + Ok(None) => return Ok(Ok(None)), + Err(e) => return Ok(Err(e)), + }; + let Ok(journal) = serde_json::from_slice::(&prepared) else { + return Ok(Err(DiskError::FileCorrupt)); + }; + if journal.protocol_version != DELETE_RECOVERY_PROTOCOL_VERSION + || journal.volume != volume_str + || journal.object != object_str + || journal.transaction != transaction + || journal.transaction.is_nil() + || journal.data_dirs.iter().any(|id| id.is_nil() || *id == transaction) + { + return Ok(Err(DiskError::FileCorrupt)); + } + let current_exists = match regular_file_exists(&root.join(STORAGE_FORMAT_FILE)) { + Ok(exists) => exists, + Err(e) => return Ok(Err(e)), + }; + let unversioned_target_absent = journal.target_digest.is_none() && !current_exists; + if !unversioned_target_absent { + return Ok(Ok(None)); + } + let mut sole_transaction = journal.predecessors.is_empty(); + if sole_transaction { + match std::fs::read_dir(&root) { + Ok(entries) => { + for entry in entries { + let entry = entry?; + let Ok(name) = entry.file_name().into_string() else { + sole_transaction = false; + break; + }; + if let Some(id) = canonical_uuid(&name).filter(|id| !id.is_nil()) + && id != transaction + && !journal.data_dirs.contains(&id) + { + sole_transaction = false; + break; + } + } + } + Err(err) if err.kind() == ErrorKind::NotFound => {} + Err(err) => return Err(err), + } + } + let receipt = CommitReceipt { + journal: journal.clone(), + finished: false, + }; + let receipt_bytes = serde_json::to_vec(&receipt).map_err(std::io::Error::other)?; + std::fs::create_dir_all(&staging_root)?; + let mut staged = tempfile::NamedTempFile::new_in(&staging_root)?; + std::io::Write::write_all(staged.as_file_mut(), &receipt_bytes)?; + if sync && !receipt_bytes.is_empty() { + staged.as_file().sync_data()?; + } + std::fs::rename(staged.path(), &receipt_path)?; + if sync { + os::fsync_dir_std(&rollback)?; + } + Ok(Ok(Some((journal, sole_transaction, unversioned_target_absent)))) + }, + ) + .await + .map_err(to_file_error)? + } + + pub(crate) async fn try_finish_and_finalize_prepared_delete_local( + &self, + volume: &str, + object: &str, + transaction: Uuid, + journal: &DeleteRecoveryJournal, + namespace_owner: Option>, + ) -> Result { + fn ignore_not_found(res: std::io::Result<()>) -> std::io::Result<()> { + match res { + Ok(()) => Ok(()), + Err(err) if err.kind() == ErrorKind::NotFound => Ok(()), + Err(err) => Err(err), + } + } + self.reject_multipart_delete_recovery_mutation(volume, object)?; + if !safe_path(object, false) || transaction.is_nil() { + return Err(DiskError::InvalidPath); + } + let object_path = self.get_object_path(volume, object)?; + let metadata_owner = os::acquire_metadata_mutation_lease(&object_path, namespace_owner).await; + let volume_guard = os::disk_volume_mutation_lock(&self.root, volume).read_owned().await; + let owner: Option> = Some(Arc::new((metadata_owner, volume_guard))); + + let mut claims: Vec = Vec::with_capacity(journal.data_dirs.len()); + for data_dir in &journal.data_dirs { + let key = SnapshotLeaseKey { + volume: volume.to_owned(), + path: format!("{object}/{data_dir}"), + }; + let opts = DeleteOptions { + recursive: true, + old_data_dir: Some(transaction), + ..Default::default() + }; + let Some(claim) = self.claim_snapshot_delete(key, SnapshotPendingDelete::Ordinary(opts)) else { + for prior in claims { + prior.finish(false); + } + return Ok(false); + }; + claims.push(claim); + } + + let bucket_path = self.io_get_bucket_path(volume)?; + let root = self.io_get_object_path(volume, object)?; + let rollback = root.join(transaction.to_string()); + let receipt_path = rollback.join(DELETE_QUORUM_COMMITTED_FILE); + let sync = effective_durability(volume).syncs_commit_metadata(); + let lease = os::acquire_namespace_mutation_lease_with_owner(&receipt_path, owner).await; + let data_dirs = journal.data_dirs.clone(); + let prune_paths: Vec<_> = rollback + .ancestors() + .take_while(|dir| *dir != bucket_path.as_path() && dir.starts_with(&bucket_path)) + .map(Path::to_path_buf) + .collect(); + + let res = os::run_blocking_namespace_operation_with_sync(lease, sync, move || -> std::io::Result<()> { + let committed = format!("{DELETE_DATA_DIR_MARKER_PREFIX}{transaction}"); + let recovery_committed = format!("{RECOVERY_DELETE_DATA_DIR_MARKER_PREFIX}{transaction}"); + let reserved = format!("{RESERVED_DELETE_DATA_DIR_MARKER_PREFIX}{transaction}"); + for data_dir in &data_dirs { + for base in [&root, &rollback] { + let dir_path = base.join(data_dir.to_string()); + match std::fs::read_dir(&dir_path) { + Ok(entries) => { + let mut removable = Vec::new(); + let mut preserve_dir = false; + for entry in entries { + let entry = entry?; + let name = entry.file_name(); + let Some(name) = name.to_str() else { + preserve_dir = true; + continue; + }; + if entry.file_type()?.is_file() + && (delete_recovery_part_file(name) + || name == committed + || name == recovery_committed + || name == reserved + || is_delete_transaction_marker(name, RESERVED_DELETE_DATA_DIR_MARKER_PREFIX)) + { + removable.push(entry.path()); + } else { + preserve_dir = true; + } + } + for path in removable { + ignore_not_found(std::fs::remove_file(path))?; + } + if sync { + os::fsync_dir_std(&dir_path)?; + } + if !preserve_dir { + match std::fs::remove_dir(&dir_path) { + Ok(()) => {} + Err(err) if err.kind() == ErrorKind::NotFound || is_dir_not_empty_error(&err) => {} + Err(err) => return Err(err), + } + } + } + Err(err) if err.kind() == ErrorKind::NotFound => {} + Err(err) => return Err(err), + } + } + } + if !data_dirs.is_empty() && sync && root.is_dir() { + os::fsync_dir_std(&root)?; + } + for name in [ + STORAGE_FORMAT_FILE_BACKUP, + DELETE_MARKER_ROLLBACK_FILE, + TARGET, + PREPARED, + FINISHED, + ] { + ignore_not_found(std::fs::remove_file(rollback.join(name)))?; + } + if sync { + os::fsync_dir_std(&rollback)?; + } + ignore_not_found(std::fs::remove_file(&receipt_path))?; + for dir_path in &prune_paths { + if let Err(err) = std::fs::remove_dir(dir_path) { + if is_dir_not_empty_error(&err) { + break; + } + if !is_benign_object_rmdir_error(&err) { + return Err(std::io::Error::other(FileAccessDeniedWithContext { + path: dir_path.clone(), + source: err, + })); + } + } + } + Ok(()) + }) + .await; + + let ok = res.is_ok(); + for claim in claims { + claim.finish(ok); + } + res.map(|()| true).map_err(|err| to_file_error(err).into()) + } } #[cfg(not(feature = "e2e-test-hooks"))] @@ -3309,6 +3777,171 @@ mod tests { inspected } + #[tokio::test] + async fn fused_delete_recovery_survives_interrupted_prepare_and_publication() { + for published in [false, true] { + let (_temp, disk, _info, original) = fixture().await; + let transaction = Uuid::new_v4(); + let prepared = disk + .try_fused_recoverable_delete_version( + "recovery", + "object", + &request(), + &DeleteOptions { + recoverable_delete: true, + old_data_dir: Some(transaction), + ..Default::default() + }, + transaction, + None, + ) + .await + .expect("prepare fused deletion"); + assert_eq!(prepared, Some(()), "the test must exercise the fused path"); + + if published { + let (_, sole, target_absent) = disk + .try_publish_prepared_delete_local("recovery", "object", transaction, None) + .await + .expect("publish fused receipt") + .expect("fused publication must apply"); + assert!(sole && target_absent); + } + + let mut state = disk + .inspect_delete_transaction("recovery", "object", transaction) + .await + .expect("inspect persisted state after interruption"); + if published { + assert_eq!(state.state, DeleteRecoveryState::QuorumCommitted); + for action in [DeleteRecoveryAction::FinishCommit, DeleteRecoveryAction::FinalizeCommit] { + state = disk + .settle_delete_transaction("recovery", "object", transaction, state.fingerprint, action) + .await + .expect("finish published deletion through recovery"); + } + assert_eq!(state.state, DeleteRecoveryState::Absent); + assert!(disk.read_all("recovery", "object/xl.meta").await.is_err()); + } else { + assert_eq!(state.state, DeleteRecoveryState::NewPrepared); + state = disk + .settle_delete_transaction( + "recovery", + "object", + transaction, + state.fingerprint, + DeleteRecoveryAction::Rollback, + ) + .await + .expect("roll back uncommitted fused deletion"); + assert_eq!(state.state, DeleteRecoveryState::Absent); + assert_eq!( + disk.read_all("recovery", "object/xl.meta") + .await + .expect("restore original metadata") + .as_ref(), + original + ); + } + } + } + + #[tokio::test] + async fn fused_delete_recovery_preserves_or_removes_shard_after_interruption() { + use crate::erasure::coding::BitrotWriter; + + for mode in 0..3 { + let published = mode != 0; + let (_temp, disk, _info, _) = fixture().await; + let data_dir = Uuid::new_v4(); + let payload = Bytes::from_static(b"original shard"); + let mut info = test_file_info("object", Uuid::nil(), Some(data_dir), None); + info.size = i64::try_from(payload.len()).expect("payload size"); + info.erasure.block_size = payload.len(); + info.parts[0].size = payload.len(); + info.parts[0].actual_size = info.size; + let original = test_meta(info.clone()); + let mut writer = BitrotWriter::new( + std::io::Cursor::new(Vec::new()), + info.erasure.shard_size(), + HashAlgorithm::HighwayHash256S, + ); + writer.write(&payload).await.expect("encode shard"); + writer.shutdown().await.expect("finish shard encoding"); + let encoded = writer.into_inner().into_inner(); + let root = disk.io_get_object_path("recovery", "object").expect("object path"); + fs::write(root.join(STORAGE_FORMAT_FILE), &original) + .await + .expect("write shard metadata"); + let shard_dir = root.join(data_dir.to_string()); + fs::create_dir_all(&shard_dir).await.expect("create shard directory"); + fs::write(shard_dir.join("part.1"), &encoded) + .await + .expect("write original shard"); + let transaction = Uuid::new_v4(); + assert_eq!( + disk.try_fused_recoverable_delete_version( + "recovery", + "object", + &request(), + &DeleteOptions { + recoverable_delete: true, + old_data_dir: Some(transaction), + ..Default::default() + }, + transaction, + None, + ) + .await + .expect("prepare fused shard deletion"), + Some(()), + "the test must exercise the fused path" + ); + if published { + let (journal, sole, target_absent) = disk + .try_publish_prepared_delete_local("recovery", "object", transaction, None) + .await + .expect("publish fused receipt") + .expect("fused publication must apply"); + assert!(sole && target_absent); + if mode == 2 { + assert!( + disk.try_finish_and_finalize_prepared_delete_local("recovery", "object", transaction, &journal, None) + .await + .expect("finish fused deletion"), + "the fused finalization must apply" + ); + assert!(!shard_dir.exists(), "fused finalization must remove the shard"); + assert!(!root.join(STORAGE_FORMAT_FILE).exists()); + assert!(!root.join(transaction.to_string()).exists()); + continue; + } + } + let mut state = disk + .inspect_delete_transaction("recovery", "object", transaction) + .await + .expect("inspect persisted state"); + let actions = if published { + vec![DeleteRecoveryAction::FinishCommit, DeleteRecoveryAction::FinalizeCommit] + } else { + vec![DeleteRecoveryAction::Rollback] + }; + for action in actions { + state = disk + .settle_delete_transaction("recovery", "object", transaction, state.fingerprint, action) + .await + .expect("recover shard deletion"); + } + assert_eq!(state.state, DeleteRecoveryState::Absent); + if published { + assert!(!shard_dir.exists(), "committed deletion must remove the shard"); + } else { + assert_eq!(fs::read(root.join(STORAGE_FORMAT_FILE)).await.expect("restore metadata"), original); + assert_eq!(fs::read(shard_dir.join("part.1")).await.expect("restore shard"), encoded); + } + } + } + #[tokio::test] async fn delete_recovery_uncommitted_inline_rolls_back_exact_bytes() { let (_temp, disk, _info, original) = fixture().await; diff --git a/crates/ecstore/src/disk/mod.rs b/crates/ecstore/src/disk/mod.rs index e69535fa5..0e86b8e29 100644 --- a/crates/ecstore/src/disk/mod.rs +++ b/crates/ecstore/src/disk/mod.rs @@ -387,6 +387,39 @@ impl Disk { } } + pub(crate) async fn try_publish_prepared_delete_local( + &self, + volume: &str, + object: &str, + transaction: Uuid, + namespace_owner: Option>, + ) -> Result> { + match self { + Self::Local(disk) => { + disk.try_publish_prepared_delete_local(volume, object, transaction, namespace_owner) + .await + } + Self::Remote(_) => Ok(None), + } + } + + pub(crate) async fn try_finish_and_finalize_prepared_delete_local( + &self, + volume: &str, + object: &str, + transaction: Uuid, + journal: &DeleteRecoveryJournal, + namespace_owner: Option>, + ) -> Result { + match self { + Self::Local(disk) => { + disk.try_finish_and_finalize_prepared_delete_local(volume, object, transaction, journal, namespace_owner) + .await + } + Self::Remote(_) => Ok(false), + } + } + pub(crate) async fn delete_version_recoverable_with_namespace_owner( &self, volume: &str, diff --git a/crates/ecstore/src/disk/os.rs b/crates/ecstore/src/disk/os.rs index 060e24cac..51379038a 100644 --- a/crates/ecstore/src/disk/os.rs +++ b/crates/ecstore/src/disk/os.rs @@ -286,6 +286,11 @@ mod dir_fsync_failures { static FAILURES: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); + #[cfg(target_os = "linux")] + pub(super) fn has_any() -> bool { + !FAILURES.lock().is_empty() + } + pub(super) fn set(dir: &Path, kind: io::ErrorKind) { FAILURES.lock().insert(dir.to_path_buf(), kind); } @@ -323,6 +328,11 @@ pub fn set_local_dir_fsync_failure(disk: &crate::disk::Disk, volume: &str, path: Ok(()) } +#[cfg(all(target_os = "linux", any(test, feature = "test-util")))] +pub(crate) fn has_dir_fsync_test_failures() -> bool { + dir_fsync_failures::has_any() +} + /// Pause a real namespace mutation inside its physical executor. #[cfg(all(any(test, feature = "test-util"), not(windows)))] pub(crate) mod prepared_publication_test_hooks { @@ -372,6 +382,17 @@ pub(crate) mod prepared_publication_test_hooks { } } + pub(crate) fn has_any_hooks() -> bool { + if !BEFORE_PUBLICATION.lock().is_empty() { + return true; + } + #[cfg(test)] + if !RENAME_DESTINATIONS.lock().is_empty() || !HARDLINK_DESTINATIONS.lock().is_empty() { + return true; + } + false + } + #[cfg(test)] type RenameDestinationHook = Box; #[cfg(test)] @@ -758,7 +779,7 @@ fn set_file_fdatasync_group_commit_wait_for_test(wait_micros: u64) { file_fdatasync_group_commit_override::set_wait_micros(wait_micros); } -fn file_fdatasync_group_commit_enabled() -> bool { +pub(crate) fn file_fdatasync_group_commit_enabled() -> bool { #[cfg(test)] if let Some(enabled) = file_fdatasync_group_commit_override::get() { return enabled; @@ -2392,6 +2413,10 @@ pub(crate) mod file_sync_probe { configure(root, None, false) } + pub(crate) fn is_active() -> bool { + !ROOTS.read().expect("file sync probe lock poisoned").is_empty() + } + pub(super) fn set_failing(root: &Path) -> ProbeGuard { configure(root, Some(1), false) } @@ -2553,18 +2578,25 @@ fn regular_files(dir: &Path) -> io::Result> { /// Fdatasync every regular file directly inside `dir`, then fsync the directory /// itself. -#[allow( - dead_code, - reason = "reached only through sync_dir_files, whose callers are tests (backlog#1823)" +#[cfg_attr( + not(target_os = "linux"), + allow( + dead_code, + reason = "reached only through sync_dir_files, whose callers are tests (backlog#1823)" + ) )] pub fn sync_dir_files_std(dir: impl AsRef) -> io::Result<()> { - for entry in std::fs::read_dir(dir.as_ref())? { - let entry = entry?; - if entry.file_type()?.is_file() { - sync_file(&entry.path())?; - } - } - fsync_dir_std(dir) + let dir = dir.as_ref(); + let files = regular_files(dir)?; + rustfs_io_metrics::record_put_rename_fdatasync_batch(rustfs_io_metrics::PUT_RENAME_FDATASYNC_BATCH_MODE_SERIAL, files.len()); + sync_files(&files)?; + let fsync_started = rustfs_io_metrics::put_stage_timer(); + let result = fsync_dir_std(dir); + rustfs_io_metrics::record_put_object_stage_duration_from( + rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_SRC_DIR_FSYNC, + fsync_started, + ); + result } /// Async wrapper around [`sync_dir_files_std`]. Large directories flush files @@ -3808,6 +3840,23 @@ pub(crate) async fn rename_all_with_prepared_source( Ok(()) } +#[cfg(target_os = "linux")] +pub(crate) fn rename_all_with_commit_guard_on_thread(source: &Path, destination: &Path, guard: &RenameCommitGuard) -> Result<()> { + guard.directories.prepare_destination().map_err(to_file_error)?; + #[cfg(test)] + prepared_publication_test_hooks::run_rename_destination(source, destination); + #[cfg(any(test, feature = "test-util"))] + { + prepared_publication_test_hooks::run(prepared_publication_test_hooks::Stage::Rename, source); + prepared_publication_test_hooks::run(prepared_publication_test_hooks::Stage::Rename, destination); + } + #[cfg(test)] + multipart_source_syscall_failures::check(multipart_source_syscall_failures::Stage::Rename, destination) + .map_err(to_file_error)?; + guard.directories.rename(source, destination, None).map_err(to_file_error)?; + Ok(()) +} + #[cfg(not(windows))] pub(crate) async fn rename_all_with_commit_guard( src_file_path: impl AsRef, @@ -6483,6 +6532,7 @@ pub(crate) fn sync_multipart_owned_record_parents( if !directory.starts_with(source) { return Err(io::Error::new(io::ErrorKind::InvalidInput, "native record barrier escaped source")); } + #[cfg_attr(not(unix), allow(unused_mut))] let mut pinned = pin_multipart_source_directory(directory, base, publication_root)?; #[cfg(unix)] { diff --git a/crates/ecstore/src/erasure/coding/decode.rs b/crates/ecstore/src/erasure/coding/decode.rs index ad9433218..6b983c9f3 100644 --- a/crates/ecstore/src/erasure/coding/decode.rs +++ b/crates/ecstore/src/erasure/coding/decode.rs @@ -175,8 +175,10 @@ const DEFAULT_RUSTFS_GET_BITROT_DECODE_OVERLAP_ENABLE: bool = false; /// stripe via their stripe handle — only when a data shard is missing or dies. /// This halves per-GET read bytes, IOPS, and bitrot hashing on healthy /// objects with 2+2 layouts. -/// Default: false (current behavior: every live shard reader is read on every -/// stripe). +/// Default: false for raw `Erasure::decode` calls without deferred parity +/// handles; `DataBlocksFirst` reader setup attaches deferred parity handles and +/// leaves those parity reserves unengaged by default unless this env var is set +/// to `false`. const ENV_RUSTFS_GET_LOCKSTEP_DATA_SHARDS_ONLY_ENABLE: &str = "RUSTFS_GET_LOCKSTEP_DATA_SHARDS_ONLY_ENABLE"; const DEFAULT_RUSTFS_GET_LOCKSTEP_DATA_SHARDS_ONLY_ENABLE: bool = false; @@ -189,6 +191,13 @@ pub(crate) fn get_lockstep_data_shards_only_enabled() -> bool { ) } +/// Whether `DataBlocksFirst` reader setup and `with_deferred_parity_handles` +/// should keep parity shards deferred until needed on healthy GETs. +pub(crate) fn get_lockstep_data_shards_only_with_deferred_parity_enabled() -> bool { + matches!(decode_read_policy(), DecodeReadPolicy::DemandBound) + || rustfs_utils::get_env_bool(ENV_RUSTFS_GET_LOCKSTEP_DATA_SHARDS_ONLY_ENABLE, true) +} + /// Get whether bitrot-decode overlap is enabled. fn is_bitrot_decode_overlap_enabled() -> bool { rustfs_utils::get_env_bool( @@ -832,6 +841,13 @@ where pub(crate) fn with_deferred_parity_handles(mut self, mut handles: Vec>) -> Self { handles.resize_with(self.readers.len(), || None); handles.truncate(self.readers.len()); + if get_lockstep_data_shards_only_with_deferred_parity_enabled() { + for (index, handle) in handles.iter().enumerate().skip(self.data_shards) { + if handle.is_some() { + self.engaged[index] = false; + } + } + } self.deferred_handles = handles; self } @@ -2596,7 +2612,8 @@ impl Erasure { // the ordinary GET read quorum. Only the opt-in GET rollout requires a // surplus source; available surplus shards and integrity proofs are // still verified before any reconstructed copy bytes are emitted. - let require_surplus_source = reader.demand_bound_lockstep && matches!(decode_read_policy(), DecodeReadPolicy::Default); + let require_surplus_source = + get_lockstep_data_shards_only_enabled() && matches!(decode_read_policy(), DecodeReadPolicy::Default); let start = offset / self.block_size; let end = end_offset.saturating_sub(1) / self.block_size; @@ -4367,9 +4384,9 @@ mod tests { .await; } - /// Compatibility lock: with the gate off (default), the lockstep path - /// keeps the pre-backlog#923 behavior and reads every live shard — - /// including parity — on every stripe. + /// Compatibility lock: with the gate off (default), raw `Erasure::decode` + /// without deferred parity handles keeps the pre-backlog#923 behavior and + /// reads every live shard — including parity — on every stripe. #[tokio::test] #[serial_test::serial] async fn test_lockstep_default_reads_all_shards_per_stripe() { diff --git a/crates/ecstore/src/object_api/types.rs b/crates/ecstore/src/object_api/types.rs index 3a3267b86..877010ce1 100644 --- a/crates/ecstore/src/object_api/types.rs +++ b/crates/ecstore/src/object_api/types.rs @@ -144,6 +144,8 @@ pub(crate) fn namespace_lock_signal_test_fence_is_lost(signal: &Arc, Uuid, OffsetDateTime), String>; + #[derive(Debug)] pub struct ObjectLockConfigSnapshot { store_id: Option, @@ -154,6 +156,7 @@ pub struct ObjectLockConfigSnapshot { lifecycle_fence: NamespaceLockFence, _lifecycle_guard: Option, metadata_transaction_guard: Option>, + fresh_put_quota: std::sync::Mutex>, } impl ObjectLockConfigSnapshot { @@ -167,6 +170,7 @@ impl ObjectLockConfigSnapshot { lifecycle_fence: NamespaceLockFence::new(), _lifecycle_guard: None, metadata_transaction_guard: None, + fresh_put_quota: std::sync::Mutex::new(None), } } @@ -187,6 +191,7 @@ impl ObjectLockConfigSnapshot { lifecycle_fence: NamespaceLockFence::new(), _lifecycle_guard: None, metadata_transaction_guard: None, + fresh_put_quota: std::sync::Mutex::new(None), } } @@ -212,6 +217,7 @@ impl ObjectLockConfigSnapshot { lifecycle_fence, _lifecycle_guard: Some(lifecycle_guard), metadata_transaction_guard: Some(Arc::new(metadata_transaction_guard)), + fresh_put_quota: std::sync::Mutex::new(None), } } @@ -233,9 +239,23 @@ impl ObjectLockConfigSnapshot { lifecycle_fence, _lifecycle_guard: None, metadata_transaction_guard: Some(metadata_transaction_guard), + fresh_put_quota: std::sync::Mutex::new(None), } } + pub(crate) fn set_fresh_put_quota(&self, quota: FreshPutQuota) { + if self._lifecycle_guard.is_some() && self.metadata_transaction_guard.is_some() { + *self.fresh_put_quota.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = Some(quota); + } + } + + pub(crate) fn take_fresh_put_quota(&self) -> Option { + self.fresh_put_quota + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .take() + } + #[allow(dead_code, reason = "snapshot-scope predicate asserted by this file's tests (backlog#1823)")] pub(crate) fn is_for_store_bucket( &self, diff --git a/crates/ecstore/src/set_disk/core/io_primitives.rs b/crates/ecstore/src/set_disk/core/io_primitives.rs index 9b389f94c..904b56810 100644 --- a/crates/ecstore/src/set_disk/core/io_primitives.rs +++ b/crates/ecstore/src/set_disk/core/io_primitives.rs @@ -1288,7 +1288,7 @@ async fn try_create_bitrot_readers_via_batch_pread( // parity readers unopened; preading them here would discard their bytes // when fill_deferred_bitrot_readers installs stripe-aligned reserves. let data_first = strategy != BitrotReaderSetupStrategy::AllShards - && crate::erasure::coding::decode::get_lockstep_data_shards_only_enabled(); + && crate::erasure::coding::decode::get_lockstep_data_shards_only_with_deferred_parity_enabled(); let mut setup = BitrotReaderSetup::new(disks.len()); for (idx, disk) in disks.iter().enumerate() { if disk.is_none() { @@ -2549,6 +2549,8 @@ pub(in crate::set_disk) struct RenameTailCleanup { pub(in crate::set_disk) struct RenameTailOutcome { pub(in crate::set_disk) convergence: RenameConvergence, pub(in crate::set_disk) cleanup: Vec, + pub(in crate::set_disk) old_current_source_all_successful_checked: bool, + pub(in crate::set_disk) all_online_local_succeeded: bool, } const RENAME_TAIL_CLEANUP_COUNTERFACTUAL_SKIP_ENV: &str = "RUSTFS_PUT_RENAME_TAIL_CLEANUP_COUNTERFACTUAL_SKIP"; @@ -3572,7 +3574,7 @@ pub(in crate::set_disk) async fn finish_rename_tail_heal< Guards: Send + 'static, Finalize: FnOnce() -> FinalizeFuture + Send, FinalizeFuture: Future + Send, - Cleanup: FnOnce(Guards, Vec) -> CleanupFuture + Send, + Cleanup: FnOnce(Guards, Vec, bool, bool) -> CleanupFuture + Send, CleanupFuture: Future + Send, Cleanup: 'static, CleanupFuture: 'static, @@ -3580,34 +3582,41 @@ pub(in crate::set_disk) async fn finish_rename_tail_heal< SubmitFuture: Future + Send, { let tail_async_started = rustfs_io_metrics::put_stage_timer(); - let (needs_heal, tail_cleanup, tail_complete) = match tail_drain.await { - Ok(Some(outcome)) => (outcome.convergence.needs_heal(), outcome.cleanup, true), - Ok(None) => { - warn!( - event = EVENT_SET_DISK_RENAME_TAIL_DRAIN_FAILED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_SET_DISK, - state = "missing_convergence", - bucket = %request.bucket, - object = request.object_prefix.as_deref().unwrap_or_default(), - "committed rename tail drain completed without convergence" - ); - (true, Vec::new(), false) - } - Err(err) => { - warn!( - event = EVENT_SET_DISK_RENAME_TAIL_DRAIN_FAILED, - component = LOG_COMPONENT_ECSTORE, - subsystem = LOG_SUBSYSTEM_SET_DISK, - state = "failed", - bucket = %request.bucket, - object = request.object_prefix.as_deref().unwrap_or_default(), - error = %err, - "rename tail drain failed" - ); - (true, Vec::new(), false) - } - }; + let (needs_heal, tail_cleanup, tail_complete, old_current_source_all_successful_checked, all_online_local_succeeded) = + match tail_drain.await { + Ok(Some(outcome)) => ( + outcome.convergence.needs_heal(), + outcome.cleanup, + true, + outcome.old_current_source_all_successful_checked, + outcome.all_online_local_succeeded, + ), + Ok(None) => { + warn!( + event = EVENT_SET_DISK_RENAME_TAIL_DRAIN_FAILED, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_SET_DISK, + state = "missing_convergence", + bucket = %request.bucket, + object = request.object_prefix.as_deref().unwrap_or_default(), + "committed rename tail drain completed without convergence" + ); + (true, Vec::new(), false, false, false) + } + Err(err) => { + warn!( + event = EVENT_SET_DISK_RENAME_TAIL_DRAIN_FAILED, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_SET_DISK, + state = "failed", + bucket = %request.bucket, + object = request.object_prefix.as_deref().unwrap_or_default(), + error = %err, + "rename tail drain failed" + ); + (true, Vec::new(), false, false, false) + } + }; rustfs_io_metrics::record_put_object_stage_duration_from( rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_TAIL_ASYNC_WAIT, tail_async_started, @@ -3651,7 +3660,12 @@ pub(in crate::set_disk) async fn finish_rename_tail_heal< let job = RenameTailCleanupJob { enqueued_at: Instant::now(), targets, - future: Box::pin(cleanup(guards, tail_cleanup)), + future: Box::pin(cleanup( + guards, + tail_cleanup, + old_current_source_all_successful_checked, + all_online_local_succeeded, + )), }; if let Err(job) = rename_tail_cleanup_queue().try_enqueue(job).await { rustfs_io_metrics::record_put_object_stage_duration_from( @@ -3676,7 +3690,13 @@ pub(in crate::set_disk) async fn finish_rename_tail_heal< cleanup_submit_started, ); let cleanup_started = rustfs_io_metrics::put_stage_timer(); - cleanup(guards, tail_cleanup).await; + cleanup( + guards, + tail_cleanup, + old_current_source_all_successful_checked, + all_online_local_succeeded, + ) + .await; rustfs_io_metrics::record_put_object_stage_duration_from( rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_TAIL_CLEANUP, cleanup_started, @@ -4155,7 +4175,14 @@ impl SetDisks { } } - if !sent_commit && success_count >= write_quorum { + let pending_tasks = tasks.len(); + let checked_count = old_current_source_checked.iter().filter(|checked| **checked).count(); + let max_source_votes = Self::max_common_old_current_source_votes(&old_current_sources); + let can_change_old_current_source = (checked_count < write_quorum + && checked_count + pending_tasks >= write_quorum) + || (max_source_votes < write_quorum && max_source_votes + pending_tasks >= write_quorum); + + if !sent_commit && success_count >= write_quorum && !can_change_old_current_source { let snapshot_commit = Self::rename_data_commit_from_observations( RenameDataObservations { disks: &coordinator_disks, @@ -4174,6 +4201,8 @@ impl SetDisks { cleanup_at_snapshot = commit.cleanup_disks.iter().map(Option::is_some).collect(); } if let Some(commit_tx) = commit_tx.take() { + #[cfg(test)] + rename_fanout_barrier::wait_for_armed_rename_before_early_ack(&fanout_dst_object).await; let _ = commit_tx.send(snapshot_commit); } sent_commit = true; @@ -4296,9 +4325,21 @@ impl SetDisks { } } + let old_current_source_all_successful_checked = errs + .iter() + .enumerate() + .filter(|(_, err)| err.is_none()) + .all(|(idx, _)| old_current_source_checked.get(idx).copied().unwrap_or(false)); + let all_online_local_succeeded = coordinator_disks.iter().zip(errs.iter()).all(|(disk, err)| match disk { + Some(d) => d.is_local() && err.is_none(), + None => true, + }); + Some(RenameTailOutcome { convergence: final_convergence, cleanup: tail_cleanup, + old_current_source_all_successful_checked, + all_online_local_succeeded, }) } }); @@ -4863,6 +4904,14 @@ impl SetDisks { }) } + pub(in crate::set_disk) fn max_common_old_current_source_votes(old_current_sources: &[Option]) -> usize { + let mut counts: HashMap<[u8; 32], usize> = HashMap::new(); + for source in old_current_sources.iter().flatten() { + *counts.entry(Self::file_info_quorum_hash(source)).or_insert(0) += 1; + } + counts.into_values().max().unwrap_or(0) + } + pub(in crate::set_disk) fn reduce_common_old_current_source( old_current_sources: &[Option], write_quorum: usize, @@ -7422,6 +7471,34 @@ pub(crate) mod rename_fanout_barrier { } } + pub(in crate::set_disk) async fn wait_for_armed_rename_before_early_ack(object: &str) { + loop { + let (any_armed, any_unpaused) = { + let reg = lock(); + let mut any_armed = false; + let mut any_unpaused = false; + for ((obj, _, phase), armed) in ®.armed { + if obj == object && *phase == Phase::Rename { + any_armed = true; + if !armed.paused.load(Ordering::SeqCst) { + any_unpaused = true; + break; + } + } + } + (any_armed, any_unpaused) + }; + if !any_armed { + return; + } + if !any_unpaused { + tokio::task::yield_now().await; + return; + } + tokio::task::yield_now().await; + } + } + /// RAII scope that observes in-flight fan-out task counts for a single /// `object`. Counts accrue only while the scope is alive; it clears its own /// entry on drop. @@ -8488,9 +8565,9 @@ mod tests { } assert_eq!( recorder.counter_value("rustfs_io_get_object_local_batch_shards_total", &[("outcome", "success")]), - 4 + 2 ); - assert_eq!(recorder.counter_value("rustfs_io_get_object_local_batch_read_bytes_total", &[]), 14); + assert_eq!(recorder.counter_value("rustfs_io_get_object_local_batch_read_bytes_total", &[]), 7); assert_eq!(recorder.histogram_sample_count("rustfs_io_get_object_local_batch_prepare_seconds"), 8); } @@ -10915,7 +10992,7 @@ mod tests { move || async move { *finalize_captured.lock().expect("finalize recorder should not poison") = true; }, - move |(), _| async move { + move |(), _, _, _| async move { *cleanup_captured.lock().expect("cleanup recorder should not poison") = true; }, |request| async move { diff --git a/crates/ecstore/src/set_disk/ops/delete_recovery.rs b/crates/ecstore/src/set_disk/ops/delete_recovery.rs index 1da6f6772..8cc5a22a5 100644 --- a/crates/ecstore/src/set_disk/ops/delete_recovery.rs +++ b/crates/ecstore/src/set_disk/ops/delete_recovery.rs @@ -97,6 +97,14 @@ pub(crate) mod test_hooks { .push((bucket.to_owned(), object.to_owned(), hook)); } + pub(super) fn has_before_gc_finalize(bucket: &str, object: &str) -> bool { + BEFORE_GC_FINALIZE + .lock() + .expect("GC finalize hooks") + .iter() + .any(|(hook_bucket, hook_object, _)| hook_bucket == bucket && hook_object == object) + } + pub(super) async fn run_before_gc_finalize(bucket: &str, object: &str) { let hook = { let mut hooks = BEFORE_GC_FINALIZE.lock().expect("GC finalize hooks"); @@ -418,6 +426,59 @@ impl SetDisks { .unwrap_or_else(|| self.ctx.begin_namespace_commit()); context.namespace_owner = Some(Arc::new((owner, Arc::clone(&transaction_guard)))); let can_commit = context.can_commit; + let can_try_local_fast_path = disks.len() == self.set_drive_count + && disks.iter().all(|d| d.as_ref().is_some_and(|d| d.is_local())) + && !crate::disk::local::has_delete_version_test_failpoints() + && { + #[cfg(test)] + { + !test_hooks::has_before_gc_finalize(bucket, object) + } + #[cfg(not(test))] + { + true + } + }; + if can_try_local_fast_path && can_commit() { + let fast_published = join_all(disks.iter().map(|disk| async { + if !can_commit() { + return Err(DiskError::other("delete recovery namespace fence was lost")); + } + let disk = disk.as_ref().ok_or(DiskError::DiskNotFound)?; + disk.try_publish_prepared_delete_local(bucket, object, transaction, context.namespace_owner.clone()) + .await + })) + .await; + let all_fast_published = fast_published.len() == self.set_drive_count + && fast_published.iter().all(|res| matches!(res, Ok(Some((_, true, true))))); + if all_fast_published && can_commit() { + #[cfg(test)] + test_hooks::record_cleanup_attempt(bucket, object); + let finalized = join_all(disks.iter().zip(fast_published.iter()).map(|(disk, pub_res)| { + let ns_owner = context.namespace_owner.clone(); + async move { + if !can_commit() { + return Ok(false); + } + let Some(disk) = disk.as_ref() else { + return Ok(false); + }; + let Ok(Some((journal, _, _))) = pub_res else { + return Ok(false); + }; + disk.try_finish_and_finalize_prepared_delete_local(bucket, object, transaction, journal, ns_owner) + .await + } + })) + .await; + if finalized.iter().all(|res| matches!(res, Ok(true))) { + if let Some(ticket) = context.recovery_ticket.as_ref() { + self.ctx.delete_recovery_hints().ack(ticket); + } + return Ok(()); + } + } + } let states = inspect(disks, bucket, object, transaction).await; let untouched_absence = |state: &Result| matches!(state, Ok(state) if state.state == DeleteRecoveryState::Absent && state.current_digest.is_none()); if states.iter().filter(|state| untouched_absence(state)).count() >= write_quorum diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index ea0e5ebc4..90b7bd3ee 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -158,13 +158,15 @@ fn rename_tail_cleanup_zero_target_tmp_delete_skip_enabled() -> bool { } fn tier_free_version_source_reuse_rename_data_enabled() -> bool { - static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); - *ENABLED.get_or_init(|| { - matches!( - std::env::var(TIER_FREE_VERSION_SOURCE_REUSE_RENAME_DATA_ENV).ok().as_deref(), - Some("1" | "true" | "TRUE" | "yes" | "YES" | "on" | "ON") - ) - }) + #[cfg(test)] + { + rustfs_utils::get_env_bool(TIER_FREE_VERSION_SOURCE_REUSE_RENAME_DATA_ENV, true) + } + #[cfg(not(test))] + { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| rustfs_utils::get_env_bool(TIER_FREE_VERSION_SOURCE_REUSE_RENAME_DATA_ENV, true)) + } } fn tier_free_version_source_reuse_rename_data_assume_homogeneous_enabled() -> bool { @@ -5523,8 +5525,10 @@ impl SetDisks { ); let tier_free_version_source_lookup_started = rustfs_io_metrics::put_stage_timer(); + let all_commit_disks_local = commit_disks.iter().flatten().all(|disk| disk.is_local()); let reuse_rename_data_source_for_tier_free_version = tier_free_version_source_reuse_rename_data_enabled() - && (commit_set.tier_free_version_rename_data_source_capable() + && (all_commit_disks_local + || commit_set.tier_free_version_rename_data_source_capable() || tier_free_version_source_reuse_rename_data_assume_homogeneous_enabled()); let mut put_tier_free_version_source = if commit_put_tier_free_version_id.is_some() && commit_tier_free_version_receipt_sink.is_some() @@ -5630,9 +5634,11 @@ impl SetDisks { ); let lock_held_tail_handoff_started = rustfs_io_metrics::put_stage_timer(); if let Ok(rename_commit) = rename_result.as_mut() { - commit_set.observe_tier_free_version_rename_data_source_capability( - rename_commit.tail_drain.is_none() && rename_commit.old_current_source_all_successful_checked, - ); + if rename_commit.tail_drain.is_none() || !rename_commit.old_current_source_all_successful_checked { + commit_set.observe_tier_free_version_rename_data_source_capability( + rename_commit.tail_drain.is_none() && rename_commit.old_current_source_all_successful_checked, + ); + } commit_set.record_capacity_scope_if_needed(commit_capacity_scope_token, &rename_commit.capacity_disks); // Install the tail watcher before any post-commit await. The // latch keeps namespace guards through their prior handoff point. @@ -5700,7 +5706,12 @@ impl SetDisks { decommission_object_lock_guard, decommission_capacity_guard, ), - targets| { + targets, + old_current_source_all_successful_checked, + all_online_local_succeeded| { + cleanup_set.observe_tier_free_version_rename_data_source_capability( + old_current_source_all_successful_checked, + ); drop(object_lock_guard); drop(write_context); drop(publication_guard); @@ -5727,7 +5738,10 @@ impl SetDisks { ) .await; let tmp_delete_started = rustfs_io_metrics::put_stage_timer(); - if zero_target_cleanup && rename_tail_cleanup_zero_target_tmp_delete_skip_enabled() { + if zero_target_cleanup + && (all_online_local_succeeded + || rename_tail_cleanup_zero_target_tmp_delete_skip_enabled()) + { rustfs_io_metrics::record_put_object_stage_duration( rustfs_io_metrics::PUT_STAGE_SET_DISK_RENAME_TAIL_CLEANUP_TMP_DELETE_COUNTERFACTUAL_SKIP, 1.0, diff --git a/crates/ecstore/src/store/mod.rs b/crates/ecstore/src/store/mod.rs index ada405f20..7dc23d234 100644 --- a/crates/ecstore/src/store/mod.rs +++ b/crates/ecstore/src/store/mod.rs @@ -1388,6 +1388,18 @@ impl crate::storage_api_contracts::object::ObjectIO for ECStore { } impl ECStore { + pub(crate) fn all_disks_local_and_online(&self) -> bool { + !self.disk_map.is_empty() + && self.disk_map.values().all(|disks| { + !disks.is_empty() + && disks.iter().all(|disk| { + disk.as_ref().is_some_and(|d| { + d.is_local() && d.runtime_state() == crate::disk::health_state::RuntimeDriveHealthState::Online + }) + }) + }) + } + /// `put_object` plus the rename_data old-size backfill /// (rustfs/backlog#1009); see `SetDisks::put_object_with_old_current_size`. /// Post-write hooks (immediate ILM transition enqueue, list-cache @@ -1400,19 +1412,36 @@ impl ECStore { data: &mut PutObjReader, opts: &ObjectOptions, ) -> Result<(ObjectInfo, Option)> { - let result = match self.handle_put_object(bucket, object, data, opts).await { - Ok((object_info, old_current_size)) => { - enqueue_transition_after_write(self, Ok(object_info), LcEventSrc::S3PutObject, opts) - .await - .map(|object_info| (object_info, old_current_size)) - } - Err(err) => Err(err), - }; + let result = self.handle_put_object(bucket, object, data, opts).await; if result.is_ok() { list_objects::observe_scanner_namespace_mutations(bucket, 1); } result } + + #[allow(clippy::too_many_arguments)] + pub async fn copy_object_with_old_current_size( + &self, + src_bucket: &str, + src_object: &str, + dst_bucket: &str, + dst_object: &str, + src_info: &mut ObjectInfo, + src_opts: &ObjectOptions, + dst_opts: &ObjectOptions, + ) -> Result<(ObjectInfo, Option)> { + let (oi, old_current_size) = box_copy_object_handler(move || { + self.handle_copy_object(src_bucket, src_object, dst_bucket, dst_object, src_info, src_opts, dst_opts) + }) + .await?; + let result = enqueue_transition_after_write(self, Ok(oi), LcEventSrc::S3CopyObject, dst_opts) + .await + .map(|oi| (oi, old_current_size)); + if result.is_ok() { + list_objects::observe_scanner_namespace_mutations(dst_bucket, 1); + } + result + } } lazy_static! { @@ -1490,20 +1519,9 @@ impl crate::storage_api_contracts::object::ObjectOperations for ECStore { src_opts: &ObjectOptions, dst_opts: &ObjectOptions, ) -> Result { - let result = enqueue_transition_after_write( - self, - box_copy_object_handler(move || { - self.handle_copy_object(src_bucket, src_object, dst_bucket, dst_object, src_info, src_opts, dst_opts) - }) - .await, - LcEventSrc::S3CopyObject, - dst_opts, - ) - .await; - if result.is_ok() { - list_objects::observe_scanner_namespace_mutations(dst_bucket, 1); - } - result + self.copy_object_with_old_current_size(src_bucket, src_object, dst_bucket, dst_object, src_info, src_opts, dst_opts) + .await + .map(|(oi, _)| oi) } #[instrument(skip(self))] @@ -1763,15 +1781,10 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for ECStore { uploaded_parts: Vec, opts: &ObjectOptions, ) -> Result { - let result = enqueue_transition_after_write( - self.as_ref(), - self.clone() - .handle_complete_multipart_upload(bucket, object, upload_id, uploaded_parts, opts) - .await, - LcEventSrc::S3CompleteMultipartUpload, - opts, - ) - .await; + let result = self + .clone() + .handle_complete_multipart_upload(bucket, object, upload_id, uploaded_parts, opts) + .await; if result.is_ok() { list_objects::observe_scanner_namespace_mutations(bucket, 1); } diff --git a/crates/ecstore/src/store/multipart.rs b/crates/ecstore/src/store/multipart.rs index 3646ae5cd..a131e50ad 100644 --- a/crates/ecstore/src/store/multipart.rs +++ b/crates/ecstore/src/store/multipart.rs @@ -640,6 +640,19 @@ impl ECStore { return Ok((res, idx, opts.expected_bucket_incarnation_id)); } + if self.single_pool() && !self.is_suspended(0).await && !self.is_pool_rebalancing(0).await { + self.apply_decommission_target_mutation_fence(0, object, &mut opts, mutation_fence) + .await; + let expected_bucket_incarnation_id = opts.expected_bucket_incarnation_id; + let lock_object = encode_dir_object(object); + let res = self + .run_external_decommission_capacity_object_mutation(0, bucket, &lock_object, object, opts, |opts| async move { + self.pools[0].new_multipart_upload(bucket, object, &opts).await + }) + .await?; + return Ok((res, 0, expected_bucket_incarnation_id)); + } + for (idx, pool) in self.pools.iter().enumerate() { if self.is_suspended(idx).await || self.is_pool_rebalancing(idx).await { continue; @@ -1167,10 +1180,11 @@ impl ECStore { let opts = &opts; if self.single_pool() { - return self.pools[0] + let result = self.pools[0] .clone() .complete_multipart_upload(bucket, object, upload_id, uploaded_parts, opts) .await; + return enqueue_transition_after_write(self.as_ref(), result, LcEventSrc::S3CompleteMultipartUpload, opts).await; } if opts.http_preconditions.is_some() && !opts.data_movement @@ -1179,7 +1193,7 @@ impl ECStore { return Err(err); } - match self.multipart_upload_pool_idx(bucket, object, upload_id, opts).await { + let result = match self.multipart_upload_pool_idx(bucket, object, upload_id, opts).await { Ok(pool_idx) => { self.pools[pool_idx] .clone() @@ -1190,21 +1204,29 @@ impl ECStore { // the pool that published it; other pools answer InvalidUploadID. Err(err) if is_err_invalid_upload_id(&err) => { let mut missing = err; + let mut found = None; for pool_idx in self.existing_multipart_pool_order().await { match self.pools[pool_idx] .clone() .complete_multipart_upload(bucket, object, upload_id, uploaded_parts.clone(), opts) .await { - Ok(info) => return Ok(info), + Ok(info) => { + found = Some(Ok(info)); + break; + } Err(err) if is_err_invalid_upload_id(&err) => missing = err, - Err(err) => return Err(err), + Err(err) => { + found = Some(Err(err)); + break; + } } } - Err(missing) + found.unwrap_or(Err(missing)) } Err(err) => Err(err), - } + }; + enqueue_transition_after_write(self.as_ref(), result, LcEventSrc::S3CompleteMultipartUpload, opts).await } #[cfg(all(test, feature = "test-util"))] @@ -1326,6 +1348,7 @@ impl ECStore { // Data movement already owns the pool-meta write lease. Forward its // loss signal into SetDisks so commit fencing observes the same lease // without trying to reacquire the namespace. + let mut transition_opts = opts.clone(); let result = self .run_decommission_capacity_admitted_mutation_with_capacity_lease( target_pool_idx, @@ -1341,8 +1364,9 @@ impl ECStore { ) .await; drop(publication_guard); + transition_opts.no_lock = request_opts.no_lock; let result = - enqueue_transition_after_write(self.as_ref(), result, LcEventSrc::S3CompleteMultipartUpload, request_opts).await; + enqueue_transition_after_write(self.as_ref(), result, LcEventSrc::S3CompleteMultipartUpload, &transition_opts).await; if result.is_ok() { list_objects::observe_scanner_namespace_mutations(bucket, 1); } diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 2e54400cb..160760a63 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -31,7 +31,7 @@ use crate::bucket::lifecycle::{ }; use crate::bucket::metadata_sys::{ acquire_bucket_metadata_transaction_read_lock_in, get_cached_bucket_incarnation_id_in, get_delete_config_from_disk_in, - get_object_lock_config_and_incarnation_from_disk_in, + get_object_lock_and_quota_config_from_disk_with_guard_in, get_object_lock_config_and_incarnation_from_disk_with_guard_in, }; use crate::bucket::object_lock::objectlock_sys::{ check_object_lock_for_deletion_with_state, ensure_recursive_force_delete_allowed_for_state, @@ -3162,7 +3162,7 @@ impl ECStore { let lifecycle_guard = self.acquire_bucket_lifecycle_read_lock(bucket).await?; let metadata_guard = acquire_bucket_metadata_transaction_read_lock_in(&self.ctx, bucket).await?; let (state, bucket_incarnation_id, config_revision) = - get_object_lock_config_and_incarnation_from_disk_in(&self.ctx, bucket).await?; + get_object_lock_config_and_incarnation_from_disk_with_guard_in(&self.ctx, bucket, Some(&lifecycle_guard)).await?; if lifecycle_guard.is_lock_lost() || metadata_guard.is_lock_lost() { return Err(Error::other("bucket lifecycle lock was lost while loading the Object Lock snapshot")); } @@ -3177,6 +3177,30 @@ impl ECStore { ))) } + async fn object_lock_config_snapshot_for_single_put(&self, bucket: &str) -> Result> { + check_valid_bucket_name(bucket)?; + let lifecycle_guard = self.acquire_bucket_lifecycle_read_lock(bucket).await?; + let metadata_guard = acquire_bucket_metadata_transaction_read_lock_in(&self.ctx, bucket).await?; + let (state, bucket_incarnation_id, config_revision, quota_res) = + get_object_lock_and_quota_config_from_disk_with_guard_in(&self.ctx, bucket, Some(&lifecycle_guard)).await?; + if lifecycle_guard.is_lock_lost() || metadata_guard.is_lock_lost() { + return Err(Error::other("bucket lifecycle lock was lost while loading the Object Lock snapshot")); + } + let snapshot = ObjectLockConfigSnapshot::for_guarded_store_bucket( + self.id, + bucket, + bucket_incarnation_id, + config_revision, + state, + lifecycle_guard, + metadata_guard, + ); + if !self.ctx.lock_manager().is_disabled() { + snapshot.set_fresh_put_quota(quota_res); + } + Ok(Arc::new(snapshot)) + } + pub(super) async fn object_lock_config_snapshot_under_lifecycle_fence( &self, bucket: &str, @@ -3187,7 +3211,7 @@ impl ECStore { } let metadata_guard = acquire_bucket_metadata_transaction_read_lock_in(&self.ctx, bucket).await?; let (state, bucket_incarnation_id, config_revision) = - get_object_lock_config_and_incarnation_from_disk_in(&self.ctx, bucket).await?; + get_object_lock_config_and_incarnation_from_disk_with_guard_in(&self.ctx, bucket, Some(&metadata_guard)).await?; if lifecycle_fence.is_lock_lost() || metadata_guard.is_lock_lost() { return Err(Error::other("bucket lock was lost while loading the Object Lock snapshot")); } @@ -4652,7 +4676,7 @@ impl ECStore { .expected_bucket_incarnation_id .ok_or_else(|| Error::other("destructive PUT is missing its bucket incarnation"))?; if opts.object_lock_config_snapshot.is_none() { - opts.object_lock_config_snapshot = Some(self.object_lock_config_snapshot(bucket).await?); + opts.object_lock_config_snapshot = Some(self.object_lock_config_snapshot_for_single_put(bucket).await?); } let snapshot = match opts.object_lock_config_snapshot.as_ref() { Some(snapshot) if snapshot.is_valid_for_destructive_put(self.id, bucket, expected_incarnation_id) => { @@ -4741,6 +4765,7 @@ impl ECStore { self.apply_decommission_target_mutation_fence(idx, object.as_str(), &mut opts, mutation_fence.or(fixed_read_anchor)) .await; let expected_data_bytes = usize::try_from(data.size()).ok(); + let mut transition_opts = opts.clone(); let result = self .run_decommission_capacity_admitted_mutation_with_capacity_lease( idx, @@ -4772,7 +4797,8 @@ impl ECStore { }, ) .await; - let result = enqueue_transition_after_write(self, result, LcEventSrc::S3PutObject, request_opts).await; + transition_opts.no_lock = request_opts.no_lock; + let result = enqueue_transition_after_write(self, result, LcEventSrc::S3PutObject, &transition_opts).await; if result.is_ok() { list_objects::observe_scanner_namespace_mutations(bucket, 1); } @@ -4788,7 +4814,7 @@ impl ECStore { data: &mut PutObjReader, opts: &ObjectOptions, ) -> Result<(ObjectInfo, Option)> { - let (object, opts) = self.prepare_put_object(bucket, object, opts).await?; + let (object, opts) = Box::pin(self.prepare_put_object(bucket, object, opts)).await?; let idx = self .select_put_object_pool_idx(bucket, object.as_str(), data.size(), &opts) .await?; @@ -4797,11 +4823,16 @@ impl ECStore { opts.decommission_capacity_admission = crate::bucket::metadata_sys::object_store_if_initialized_in(&self.ctx).await; } let receipt_sink = install_tier_free_version_receipt_sink(&mut opts); - let result = self.pools[idx] - .put_object_with_old_current_size(bucket, object.as_str(), data, &opts) - .await; + let result = Box::pin(self.pools[idx].put_object_with_old_current_size(bucket, object.as_str(), data, &opts)).await; enqueue_recorded_tier_free_versions(self, receipt_sink).await; - result + match result { + Ok((object_info, old_current_size)) => { + Box::pin(enqueue_transition_after_write(self, Ok(object_info), LcEventSrc::S3PutObject, &opts)) + .await + .map(|object_info| (object_info, old_current_size)) + } + Err(err) => Err(err), + } } #[instrument(level = "trace", skip(self))] @@ -4859,7 +4890,7 @@ impl ECStore { src_info: &mut ObjectInfo, src_opts: &ObjectOptions, dst_opts: &ObjectOptions, - ) -> Result { + ) -> Result<(ObjectInfo, Option)> { check_copy_obj_args(src_bucket, src_object)?; if src_bucket == dst_bucket && src_object == dst_object { // In-place copies (metadata replace, restore) rewrite a key that @@ -4938,9 +4969,17 @@ impl ECStore { }; if cp_src_dst_same { - let (_, pool_idx) = self - .get_latest_accessible_object_info_with_idx(src_bucket, &src_object, &version_aware_lookup_opts(src_opts, true)) - .await?; + let pool_idx = if self.single_pool() { + 0 + } else { + self.get_latest_accessible_object_info_with_idx( + src_bucket, + &src_object, + &version_aware_lookup_opts(src_opts, true), + ) + .await? + .1 + }; let source_pool_writable = !self.is_suspended(pool_idx).await && !self.is_pool_rebalancing(pool_idx).await; if source_pool_writable @@ -4961,7 +5000,8 @@ impl ECStore { .await }, ) - .await; + .await + .map(|oi| (oi, None)); } if source_pool_writable && !dst_opts.versioned && src_opts.version_id.is_none() { @@ -4989,7 +5029,8 @@ impl ECStore { .await }, ) - .await; + .await + .map(|oi| (oi, None)); } // Transitioned object self-copy: restore from tier into the same pool. let mut put_opts = ObjectOptions { @@ -5017,7 +5058,7 @@ impl ECStore { return if let Some(reader) = src_info.put_object_reader.as_mut() { let receipt_sink = install_tier_free_version_receipt_sink(&mut put_opts); let result = self.pools[pool_idx] - .put_object(dst_bucket, &dst_object, reader, &put_opts) + .put_object_with_old_current_size(dst_bucket, &dst_object, reader, &put_opts) .await; enqueue_recorded_tier_free_versions(self, receipt_sink).await; result @@ -5061,7 +5102,7 @@ impl ECStore { } let receipt_sink = install_tier_free_version_receipt_sink(&mut put_opts); let result = self.pools[pool_idx] - .put_object(dst_bucket, &dst_object, reader, &put_opts) + .put_object_with_old_current_size(dst_bucket, &dst_object, reader, &put_opts) .await; enqueue_recorded_tier_free_versions(self, receipt_sink).await; return result; @@ -5081,11 +5122,14 @@ impl ECStore { .await }, ) - .await; + .await + .map(|oi| (oi, None)); } } - let pool_idx = if dst_opts.no_lock { + let pool_idx = if self.single_pool() && !self.is_suspended(0).await && !self.is_pool_rebalancing(0).await { + 0 + } else if dst_opts.no_lock { self.get_pool_idx_no_lock(dst_bucket, &dst_object, src_info.size).await? } else { self.get_pool_idx(dst_bucket, &dst_object, src_info.size).await? @@ -5118,7 +5162,7 @@ impl ECStore { if let Some(put_object_reader) = src_info.put_object_reader.as_mut() { let receipt_sink = install_tier_free_version_receipt_sink(&mut put_opts); let result = self.pools[pool_idx] - .put_object(dst_bucket, dst_object_name, put_object_reader, &put_opts) + .put_object_with_old_current_size(dst_bucket, dst_object_name, put_object_reader, &put_opts) .await; enqueue_recorded_tier_free_versions(self, receipt_sink).await; return result; @@ -5350,8 +5394,9 @@ impl ECStore { opts.add_namespace_lock_guard(guard); } let current_bucket_incarnation_id = if let Some(metadata_guard) = _object_lock_metadata_guard.as_ref() { + let guard_ref = _bucket_lifecycle_guard.as_deref().unwrap_or_else(|| metadata_guard.as_ref()); let (state, incarnation_id, config_revision) = - get_delete_config_from_disk_in(&self.ctx, bucket, None, opts.lifecycle_expiry_policy.as_ref()).await?; + get_delete_config_from_disk_in(&self.ctx, bucket, Some(guard_ref), opts.lifecycle_expiry_policy.as_ref()).await?; opts.object_lock_config_snapshot = Some(Arc::new(ObjectLockConfigSnapshot::for_store_bucket_under_lifecycle_fence( self.id, bucket, @@ -5591,6 +5636,42 @@ impl ECStore { return Ok(obj); } + if self.single_pool() + && !creates_latest_marker + && !opts.data_movement + && opts.http_preconditions.is_none() + && !opts.replication_request + && !opts.versioned + && !opts.version_suspended + && opts.version_id.is_none() + && !self.is_suspended(0).await + && !self.is_pool_rebalancing(0).await + { + let pool = self.pools[0].clone(); + match self + .run_external_decommission_capacity_object_delete(0, bucket, object, object, opts.clone(), |opts| async move { + pool.delete_object(bucket, object, opts).await + }) + .await + { + Ok(mut res) => { + res.name = decode_dir_object(res.name.as_str()); + return Ok(res); + } + Err(err) if is_err_read_quorum(&err) => return Err(StorageError::ErasureWriteQuorum), + Err(err) => { + if should_purge_orphan_dir_on_missing(&err, object) && self.purge_orphan_dir_object(bucket, object).await { + return Ok(ObjectInfo { + bucket: bucket.to_owned(), + name: decode_dir_object(object), + ..Default::default() + }); + } + return Err(err); + } + } + } + // Determine which pool contains it let existing_pool_info = if creates_latest_marker { self.get_pool_info_for_delete_marker(bucket, object, &gopts).await @@ -9903,6 +9984,7 @@ mod tests { copy_store .handle_copy_object(bucket, source, bucket, target, &mut source_info, &ObjectOptions::default(), &options) .await + .map(|(oi, _)| oi) }); let deadline = tokio::time::Instant::now() + Duration::from_secs(30); tokio::time::timeout_at(deadline, barrier.wait_until_paused()) diff --git a/crates/s3select-query/Cargo.toml b/crates/s3select-query/Cargo.toml index 3fbf7778c..b786f2197 100644 --- a/crates/s3select-query/Cargo.toml +++ b/crates/s3select-query/Cargo.toml @@ -57,8 +57,6 @@ tracing = { workspace = true } [dev-dependencies] rustfs-test-utils = { workspace = true, features = ["put-object-commit-barrier"] } -# Match the streaming inference's field order in Arrow's JSON reference. -serde_json = { workspace = true, features = ["preserve_order"] } [lib] doctest = false diff --git a/crates/s3select-query/src/dispatcher/json_document_schema.rs b/crates/s3select-query/src/dispatcher/json_document_schema.rs index 1f8c33c46..0e73b2364 100644 --- a/crates/s3select-query/src/dispatcher/json_document_schema.rs +++ b/crates/s3select-query/src/dispatcher/json_document_schema.rs @@ -426,14 +426,18 @@ impl InferredObject { #[cfg(test)] mod tests { use super::*; + use crate::dispatcher::json_lines_schema; use datafusion::arrow::json::reader::infer_json_schema as arrow_infer_json_schema; const TEST_SCHEMA_LIMIT: usize = 4 * 1024 * 1024; fn assert_matches_arrow(input: &[u8]) -> Schema { let records = input.iter().filter(|byte| **byte == b'\n').count(); - let (expected, expected_records) = arrow_infer_json_schema(io::BufReader::new(input), Some(records)) + let (_, expected_records) = arrow_infer_json_schema(io::BufReader::new(input), Some(records)) .expect("Arrow should infer the compatibility fixture"); + // Arrow's types in source key order, which is what JSON LINES uses. + let expected = json_lines_schema::infer_schema(input, records) + .expect("JSON LINES inference should accept the compatibility fixture"); let (actual, actual_records, _) = infer_schema(input, records, &AtomicBool::new(false), TEST_SCHEMA_LIMIT) .expect("streaming inference should accept the compatibility fixture"); assert_eq!(actual_records, expected_records); diff --git a/crates/s3select-query/src/dispatcher/json_lines_schema.rs b/crates/s3select-query/src/dispatcher/json_lines_schema.rs new file mode 100644 index 000000000..806969ce4 --- /dev/null +++ b/crates/s3select-query/src/dispatcher/json_lines_schema.rs @@ -0,0 +1,185 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use datafusion::arrow::{ + datatypes::{DataType, FieldRef, Fields, Schema}, + error::ArrowError, + json::reader::{ValueIter, infer_json_schema_from_iterator}, +}; +use serde::de::{self, DeserializeSeed, MapAccess, SeqAccess, Visitor}; +use std::{collections::HashMap, fmt, sync::Arc}; + +/// Infers a JSON LINES schema with Arrow's types and the source's first-seen +/// key order. +/// +/// Arrow walks each record as a `serde_json::Map`, which sorts keys unless the +/// process-wide `serde_json/preserve_order` feature is enabled, so `SELECT *` +/// would otherwise reorder fields and CSV output columns. +pub(super) fn infer_schema(bytes: &[u8], max_records: usize) -> Result { + let schema = infer_json_schema_from_iterator(ValueIter::new(bytes, Some(max_records)))?; + let mut order = KeyOrder::default(); + // Same records as `ValueIter`: one value per line, blank lines skipped. + // Arrow has already rejected invalid UTF-8 and JSON within this prefix. + let records = bytes + .split(|byte| *byte == b'\n') + .filter_map(|line| std::str::from_utf8(line).ok()) + .map(str::trim) + .filter(|line| !line.is_empty()) + .take(max_records); + for record in records { + let mut deserializer = serde_json::Deserializer::from_str(record); + (&mut order) + .deserialize(&mut deserializer) + .map_err(|error| ArrowError::JsonError(format!("Not valid JSON: {error}")))?; + } + Ok(Schema::new_with_metadata(order.reorder(schema.fields()), schema.metadata().clone())) +} + +/// First-seen key order of a JSON object, merged across records. Array +/// elements share their parent's node, matching Arrow's `List` shape. +#[derive(Default)] +struct KeyOrder { + positions: HashMap, + children: Vec, +} + +impl KeyOrder { + fn child(&mut self, key: String) -> &mut KeyOrder { + let next = self.children.len(); + let position = *self.positions.entry(key).or_insert(next); + if position == next { + self.children.push(KeyOrder::default()); + } + &mut self.children[position] + } + + fn reorder(&self, fields: &Fields) -> Fields { + let mut fields: Vec = fields + .iter() + .map(|field| match self.positions.get(field.name()) { + Some(&position) => { + let data_type = self.children[position].reorder_type(field.data_type()); + Arc::new(field.as_ref().clone().with_data_type(data_type)) + } + None => Arc::clone(field), + }) + .collect(); + fields.sort_by_key(|field| self.positions.get(field.name()).copied().unwrap_or(usize::MAX)); + fields.into() + } + + fn reorder_type(&self, data_type: &DataType) -> DataType { + match data_type { + DataType::Struct(fields) => DataType::Struct(self.reorder(fields)), + DataType::List(item) => { + DataType::List(Arc::new(item.as_ref().clone().with_data_type(self.reorder_type(item.data_type())))) + } + other => other.clone(), + } + } +} + +impl<'de> DeserializeSeed<'de> for &mut KeyOrder { + type Value = (); + + fn deserialize>(self, deserializer: D) -> Result<(), D::Error> { + deserializer.deserialize_any(self) + } +} + +impl<'de> Visitor<'de> for &mut KeyOrder { + type Value = (); + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a JSON value") + } + + fn visit_map>(self, mut map: A) -> Result<(), A::Error> { + while let Some(key) = map.next_key::()? { + map.next_value_seed(self.child(key))?; + } + Ok(()) + } + + fn visit_seq>(self, mut seq: A) -> Result<(), A::Error> { + while seq.next_element_seed(&mut *self)?.is_some() {} + Ok(()) + } + + fn visit_bool(self, _: bool) -> Result<(), E> { + Ok(()) + } + + fn visit_i64(self, _: i64) -> Result<(), E> { + Ok(()) + } + + fn visit_u64(self, _: u64) -> Result<(), E> { + Ok(()) + } + + fn visit_f64(self, _: f64) -> Result<(), E> { + Ok(()) + } + + fn visit_str(self, _: &str) -> Result<(), E> { + Ok(()) + } + + fn visit_unit(self) -> Result<(), E> { + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::infer_schema; + use datafusion::arrow::datatypes::{DataType, Fields}; + + fn names(fields: &Fields) -> Vec<&str> { + fields.iter().map(|field| field.name().as_str()).collect() + } + + #[test] + fn keeps_first_seen_key_order_at_every_level() { + let input = b"{\"z\":1,\"a\":{\"y\":[{\"k2\":1,\"k1\":2}],\"b\":2}}\n\n{\"m\":null,\"a\":{\"c\":3}}\n"; + let schema = infer_schema(input, 1000).expect("schema"); + + assert_eq!(names(schema.fields()), ["z", "a", "m"]); + let DataType::Struct(a) = schema.field(1).data_type() else { + panic!("a should be a struct"); + }; + assert_eq!(names(a), ["y", "b", "c"]); + let DataType::List(item) = a[0].data_type() else { + panic!("y should be a list"); + }; + let DataType::Struct(y) = item.data_type() else { + panic!("y items should be structs"); + }; + assert_eq!(names(y), ["k2", "k1"]); + } + + #[test] + fn ignores_records_after_the_inference_prefix_like_arrow() { + let schema = infer_schema(b"{\"b\":1,\"a\":2}\nnot json\n", 1).expect("schema"); + + assert_eq!(names(schema.fields()), ["b", "a"]); + } + + #[test] + fn keeps_arrow_errors_for_invalid_records() { + assert!(infer_schema(b"{\"a\":1}\n[1]\n", 1000).is_err()); + assert!(infer_schema(b"{\"a\":\n", 1000).is_err()); + } +} diff --git a/crates/s3select-query/src/dispatcher/manager.rs b/crates/s3select-query/src/dispatcher/manager.rs index 06e21393c..d6d0db5e6 100644 --- a/crates/s3select-query/src/dispatcher/manager.rs +++ b/crates/s3select-query/src/dispatcher/manager.rs @@ -28,11 +28,12 @@ use datafusion::{ record_batch::RecordBatch, }, datasource::{ - file_format::{csv::CsvFormat, json::JsonFormat}, + file_format::{DEFAULT_SCHEMA_INFER_MAX_RECORD, csv::CsvFormat, json::JsonFormat}, listing::{ListingOptions, ListingTable, ListingTableConfig, ListingTableUrl}, }, error::Result as DFResult, execution::{RecordBatchStream, SendableRecordBatchStream}, + object_store::ObjectStoreExt as _, sql::sqlparser::parser::ParserError, }; use futures::Stream; @@ -61,7 +62,7 @@ use tokio::{ }; use crate::{ - dispatcher::{json_document_table::JsonDocumentTable, parquet_table::ParquetSelectTable}, + dispatcher::{json_document_table::JsonDocumentTable, json_lines_schema, parquet_table::ParquetSelectTable}, execution::factory::QueryExecutionFactoryRef, instance::{DEFAULT_MAX_CONCURRENT_QUERIES, DEFAULT_QUERY_TIMEOUT_SECS}, metadata::{ContextProviderExtension, MetadataProvider, TableHandleProviderRef, base_table::BaseTableProvider}, @@ -508,7 +509,11 @@ impl SimpleQueryDispatcher { return Err(SelectError::InvalidDataSource.into()); }; - let resolve_schema = listing_options.infer_schema(session.inner(), &table_path).await?; + let resolve_schema = if self.input.request.input_serialization.json.is_some() { + infer_json_lines_schema(session, &table_path).await? + } else { + listing_options.infer_schema(session.inner(), &table_path).await? + }; let config = if need_rename_volume_name { let mut new_fields = Vec::new(); for (i, field) in resolve_schema.fields().iter().enumerate() { @@ -549,6 +554,14 @@ impl SimpleQueryDispatcher { } } +/// Reads the object like DataFusion's `JsonFormat::infer_schema` does, but +/// keeps the source key order (see `json_lines_schema::infer_schema`). +async fn infer_json_lines_schema(session: &SessionCtx, table_path: &ListingTableUrl) -> DFResult { + let store = session.inner().runtime_env().object_store(table_path.object_store())?; + let bytes = store.get(table_path.prefix()).await?.bytes().await?; + Ok(Arc::new(json_lines_schema::infer_schema(&bytes, DEFAULT_SCHEMA_INFER_MAX_RECORD)?)) +} + fn is_json_document_input(input: &SelectObjectContentInput) -> bool { input .request @@ -856,6 +869,7 @@ mod tests { array::{Int32Array, StringArray}, datatypes::{DataType, Field, Schema, SchemaRef}, record_batch::RecordBatch, + util::display::array_value_to_string, }, common::DataFusionError, execution::object_store::ObjectStoreUrl, @@ -1727,6 +1741,334 @@ mod tests { } } + async fn json_lines_plan_schema(data: &'static [u8], sql: &str) -> SchemaRef { + let mut input = test_input(); + input.key = "records.json".to_owned(); + input.request.input_serialization.csv = None; + input.request.input_serialization.json = Some(JSONInput { + type_: Some(JSONType::from_static(JSONType::LINES)), + }); + let input = Arc::new(input); + let optimizer = Arc::new(CascadeOptimizerBuilder::default().build()); + let dispatcher = test_dispatcher_for_input( + Arc::clone(&input), + Arc::new(Semaphore::new(1)), + Duration::from_secs(30), + Arc::new(SqlQueryExecutionFactory::new(optimizer, Arc::new(LocalScheduler {}))), + ); + let query = Query::new(QueryContext { input }, sql.to_owned()); + let machine = dispatcher.build_query_state_machine(query).await.expect("build query state"); + let store_url = ObjectStoreUrl::parse("s3://test-bucket").expect("test store URL"); + let store = machine + .session + .inner() + .runtime_env() + .object_store(&store_url) + .expect("test store"); + store + .put(&Path::from("records.json"), data.into()) + .await + .expect("write JSON LINES object"); + let Plan::Query(plan) = dispatcher + .build_logical_plan(machine) + .await + .expect("plan JSON LINES query") + .expect("select plan"); + Arc::new(plan.df_plan.schema().as_arrow().clone()) + } + + fn field_names(fields: &datafusion::arrow::datatypes::Fields) -> Vec<&str> { + fields.iter().map(|field| field.name().as_str()).collect() + } + + fn struct_fields(data_type: &DataType) -> &datafusion::arrow::datatypes::Fields { + match data_type { + DataType::Struct(fields) => fields, + DataType::List(item) => struct_fields(item.data_type()), + other => panic!("expected a struct or list of structs, got {other}"), + } + } + + #[tokio::test] + async fn json_lines_select_star_keeps_source_field_order() { + let schema = json_lines_plan_schema( + b"{\"z\":1,\"a b\":{\"y\":true,\"B\":[{\"k2\":1,\"k1\":2}]},\"A\":\"x\"}\n\n \r\n{\"A\":\"y\",\"$c\":2,\"a b\":{\"x\":1}}\r\n", + "SELECT * FROM S3Object", + ) + .await; + + assert_eq!(field_names(schema.fields()), ["z", "a b", "A", "$c"]); + let nested = struct_fields(schema.field_with_name("a b").expect("nested field").data_type()); + assert_eq!(field_names(nested), ["y", "B", "x"]); + let listed = struct_fields(nested.find("B").expect("list field").1.data_type()); + assert_eq!(field_names(listed), ["k2", "k1"]); + } + + #[tokio::test] + async fn path_projections_are_named_after_their_last_key() { + const DATA: &[u8] = b"{\"obj0\":{\"id\":\"id0\",\"inner\":{\"Key\":1}}}\n"; + for (sql, expected) in [ + ("SELECT s.obj0.id, s.obj0.inner.\"Key\" FROM S3Object s", ["id", "Key"].as_slice()), + ("SELECT obj0.id FROM S3Object", &["id"]), + ("SELECT s.obj0.id FROM S3Object s ORDER BY s.obj0.id", &["id"]), + ("SELECT s.obj0.id, COUNT(*) FROM S3Object s GROUP BY s.obj0.id", &["id", "count(*)"]), + ] { + let schema = json_lines_plan_schema(DATA, sql).await; + assert_eq!(field_names(schema.fields()), expected, "{sql}"); + } + } + + #[tokio::test] + async fn path_projection_names_do_not_clash_with_other_columns() { + const DATA: &[u8] = b"{\"id\":\"top\",\"obj0\":{\"id\":\"id0\"}}\n"; + for sql in ["SELECT *, s.obj0.id FROM S3Object s", "SELECT s.*, s.obj0.id FROM S3Object s"] { + let schema = json_lines_plan_schema(DATA, sql).await; + assert_eq!(field_names(schema.fields()), ["id", "obj0", "s.obj0[id]"], "{sql}"); + } + for sql in [ + "SELECT s.obj0.id FROM S3Object s ORDER BY s.id", + "SELECT s.obj0.id FROM S3Object s ORDER BY id", + "SELECT s.obj0.id FROM S3Object s ORDER BY upper(s.id)", + ] { + let schema = json_lines_plan_schema(DATA, sql).await; + assert_eq!(field_names(schema.fields()), ["s.obj0[id]"], "{sql}"); + } + let schema = json_lines_plan_schema(DATA, "SELECT s.obj0.id, (s.id) FROM S3Object s").await; + assert_eq!(field_names(schema.fields()), ["s.obj0[id]", "id"]); + } + + #[tokio::test] + async fn path_aliases_do_not_collide_with_retained_path_names() { + const DATA: &[u8] = br#"{"id":"top","obj0":{"id":"a"},"obj1":{"id":"b"},"obj2":{"s.obj0[id]":"c","obj0":"d"}}"#; + for (sql, expected) in [ + ( + "SELECT s.obj0.id, s.obj1.id, s.obj2.\"s.obj0[id]\" FROM S3Object s", + vec!["s.obj0[id]", "s.obj1[id]", "s.obj2[s.obj0[id]]"], + ), + ( + "SELECT s.obj2.\"s.obj0[id]\", s.obj1.id, s.obj0.id FROM S3Object s", + vec!["s.obj2[s.obj0[id]]", "s.obj1[id]", "s.obj0[id]"], + ), + ("SELECT (s.obj0.id), s.obj2.\"s.obj0[id]\" FROM S3Object s", vec!["id", "s.obj0[id]"]), + ( + "SELECT s.obj0.id, s.obj2.\"s.obj0[id]\" FROM S3Object s ORDER BY s.id", + vec!["s.obj0[id]", "s.obj2[s.obj0[id]]"], + ), + ("SELECT s.obj2.obj0 FROM S3Object s ORDER BY s.obj0.id", vec!["s.obj2[obj0]"]), + ("SELECT s.obj2.obj0 FROM S3Object s ORDER BY (s.obj0.id)", vec!["s.obj2[obj0]"]), + ] { + let schema = json_lines_plan_schema(DATA, sql).await; + assert_eq!(field_names(schema.fields()), expected, "{sql}"); + } + } + + const PATH_NAMES_DATA: &[u8] = br#"{"id":"A","x":"B","n":1,"obj":{"lower(s.id)":7,"count(*)":3,"id":"x","ID":"X","s.n":2,"s.n + Int64(1)":3},"obj0":{"id":"a"},"obj1":{"id":"b"},"obj2":{"s.obj0[id]":"c","obj0":"d"},"a":{"id":1},"b":{"id":2}} +"#; + + /// Plans and runs `sql` over a stored JSON LINES object through the + /// production dispatcher. Returns the planned output names and the rows. + async fn production_json_lines_query(bucket: &str, key: &str, sql: &str) -> QueryResult<(Vec, Vec>)> { + let mut input = (*json_snapshot_input()).clone(); + input.bucket = bucket.to_owned(); + input.key = key.to_owned(); + input.request.expression = sql.to_owned(); + let input = Arc::new(input); + let snapshot = snapshot_test_env().await.prepare_select_object_snapshot(bucket, key).await; + let dispatcher = production_dispatcher(Arc::clone(&input)); + let query = Query::new_with_snapshot(QueryContext { input }, sql.to_owned(), snapshot); + let machine = dispatcher.build_query_state_machine(query).await?; + let plan = dispatcher + .build_logical_plan(Arc::clone(&machine)) + .await? + .expect("SELECT should produce a logical plan"); + let Plan::Query(query_plan) = &plan; + let names = query_plan + .df_plan + .schema() + .fields() + .iter() + .map(|field| field.name().clone()) + .collect(); + let Output::StreamData(stream) = dispatcher.execute_logical_plan(plan, machine).await? else { + panic!("SELECT should return rows"); + }; + let batches: Vec<_> = stream.try_collect().await?; + let rows = batches + .iter() + .flat_map(|batch| { + (0..batch.num_rows()).map(|row| { + batch + .columns() + .iter() + .map(|column| array_value_to_string(column, row).expect("display a result value")) + .collect() + }) + }) + .collect(); + Ok((names, rows)) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn production_path_alias_does_not_duplicate_a_retained_path_name() { + const BUCKET: &str = "s3select-retained-path-name"; + const KEY: &str = "retained.jsonl"; + let env = snapshot_test_env().await; + env.make_bucket(BUCKET, false).await; + env.put_object_bytes( + BUCKET, + KEY, + br#"{"obj0":{"id":"a"},"obj1":{"id":"b"},"obj2":{"s.obj0[id]":"c"}} +"# + .to_vec(), + ) + .await; + let (names, rows) = + production_json_lines_query(BUCKET, KEY, "SELECT s.obj0.id, s.obj1.id, s.obj2.\"s.obj0[id]\" FROM S3Object s") + .await + .expect("the query plans and runs"); + assert_eq!(names, ["s.obj0[id]", "s.obj1[id]", "s.obj2[s.obj0[id]]"]); + assert_eq!(rows, [["a", "b", "c"]]); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn production_path_projections_take_their_key_only_when_it_is_a_unique_output_name() { + const BUCKET: &str = "s3select-path-projection-names"; + const KEY: &str = "names.jsonl"; + let env = snapshot_test_env().await; + env.make_bucket(BUCKET, false).await; + env.put_object_bytes(BUCKET, KEY, PATH_NAMES_DATA.to_vec()).await; + let cases: &[(&str, &[&str])] = &[ + ( + "SELECT s.obj.\"lower(s.id)\", LOWER(s.id) FROM S3Object s", + &["s.obj[lower(s.id)]", "lower(s.id)"], + ), + ( + "SELECT LOWER(s.id), s.obj.\"lower(s.id)\" FROM S3Object s", + &["lower(s.id)", "s.obj[lower(s.id)]"], + ), + ( + "SELECT s.obj.\"count(*)\", COUNT(*) FROM S3Object s GROUP BY s.obj.\"count(*)\"", + &["s.obj[count(*)]", "count(*)"], + ), + ("SELECT s.obj0.id, COUNT(*) FROM S3Object s GROUP BY s.obj0.id", &["id", "count(*)"]), + ("SELECT s.obj0.id, LOWER(s.id) FROM S3Object s", &["id", "lower(s.id)"]), + ("SELECT s.obj0.id, s.x AS id FROM S3Object s", &["s.obj0[id]", "id"]), + ("SELECT s.x AS id, s.obj0.id FROM S3Object s", &["id", "s.obj0[id]"]), + ( + "SELECT *, s.obj0.id FROM S3Object s", + &["id", "x", "n", "obj", "obj0", "obj1", "obj2", "a", "b", "s.obj0[id]"], + ), + ("SELECT s.obj.ID, s.id FROM S3Object s", &["s.obj[id]", "id"]), + ("SELECT s.obj.\"ID\", s.id FROM S3Object s", &["ID", "id"]), + ("SELECT s.obj.\"s.n\", CAST(s.n AS INT) FROM S3Object s", &["s.obj[s.n]", "s.n"]), + ( + "SELECT s.n + 1, s.obj.\"s.n + Int64(1)\" FROM S3Object s", + &["s.n + Int64(1)", "s.obj[s.n + Int64(1)]"], + ), + ("SELECT s.obj0.id FROM S3Object s ORDER BY s.id", &["s.obj0[id]"]), + ("SELECT s.obj0.id FROM S3Object s ORDER BY s.obj0.id", &["id"]), + ("SELECT (s.obj0.id) FROM S3Object s", &["id"]), + ("SELECT s.obj0.id, (s.id) FROM S3Object s", &["s.obj0[id]", "id"]), + ("SELECT s.a.id, s.b.id FROM S3Object s", &["s.a[id]", "s.b[id]"]), + ("SELECT s.a.id, s.b.id, s.obj2.obj0 FROM S3Object s", &["s.a[id]", "s.b[id]", "obj0"]), + ("SELECT s.obj2.\"s.obj0[id]\", s.obj0.id FROM S3Object s", &["s.obj0[id]", "id"]), + ( + "SELECT s.obj2.\"s.obj0[id]\", s.obj1.id, s.obj0.id FROM S3Object s", + &["s.obj2[s.obj0[id]]", "s.obj1[id]", "s.obj0[id]"], + ), + ( + "SELECT s.obj2.\"s.obj0[id]\", s.obj0.id, LOWER(s.id) FROM S3Object s", + &["s.obj0[id]", "id", "lower(s.id)"], + ), + ]; + for (sql, expected) in cases { + let (names, _) = production_json_lines_query(BUCKET, KEY, sql) + .await + .unwrap_or_else(|error| panic!("{sql}: {error}")); + assert_eq!(names, *expected, "{sql}"); + } + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn production_path_projection_naming_keeps_queries_planning() { + const BUCKET: &str = "s3select-path-projection-planning"; + const KEY: &str = "planning.jsonl"; + let env = snapshot_test_env().await; + env.make_bucket(BUCKET, false).await; + env.put_object_bytes(BUCKET, KEY, PATH_NAMES_DATA.to_vec()).await; + env.put_object_bytes(BUCKET, "no-top-level-id.jsonl", br#"{"obj0":{"id":"a"},"obj1":{"id":"b"}}"#.to_vec()) + .await; + let mut failures = Vec::new(); + // GROUP BY resolves a SELECT alias when no input column has its name. + for sql in [ + "SELECT s.obj0.id FROM S3Object s GROUP BY id", + "SELECT s.obj0.id, s.obj1.id AS k FROM S3Object s GROUP BY id, k", + ] { + if let Err(error) = production_json_lines_query(BUCKET, "no-top-level-id.jsonl", sql).await { + failures.push(format!("{sql}: {error}")); + } + } + // Each query also plans when every path projection keeps DataFusion's name. + for sql in [ + "SELECT s.obj0.id FROM S3Object s", + "SELECT obj0.id FROM S3Object", + "SELECT s.obj0.id FROM S3Object s WHERE s.obj0.id = 'a'", + "SELECT s.obj0.id FROM S3Object s WHERE s.obj0.id IN ('a', 'b') LIMIT 1", + "SELECT s.obj0.id FROM S3Object s LIMIT 0", + "SELECT s.obj0.id FROM S3Object s ORDER BY s.obj0.id DESC LIMIT 1", + "SELECT s.obj0.id FROM S3Object s ORDER BY id", + "SELECT s.obj0.id FROM S3Object s ORDER BY s.id", + "SELECT s.obj0.id FROM S3Object s ORDER BY upper(s.id)", + "SELECT s.obj0.id AS k FROM S3Object s ORDER BY k", + "SELECT s.obj0.id, s.obj2.obj0 FROM S3Object s ORDER BY s.obj0.id", + "SELECT s.obj2.obj0 FROM S3Object s ORDER BY (s.obj0.id)", + "SELECT s.obj0.id, s.id FROM S3Object s ORDER BY 2", + "SELECT s.obj0.id FROM S3Object s GROUP BY s.obj0.id", + "SELECT s.obj0.id, COUNT(*) FROM S3Object s GROUP BY s.obj0.id", + "SELECT s.obj0.id, COUNT(*) AS id FROM S3Object s GROUP BY s.obj0.id", + "SELECT COUNT(*), s.obj0.id FROM S3Object s GROUP BY s.obj0.id ORDER BY COUNT(*)", + "SELECT COUNT(s.obj0.id) FROM S3Object s", + "SELECT MAX(s.obj0.id) FROM S3Object s", + "SELECT UPPER(s.obj0.id), s.obj0.id FROM S3Object s", + "SELECT s.obj0.id, s.obj0.id = 'a' FROM S3Object s", + "SELECT s.obj0.id, s.obj0.id AS x FROM S3Object s", + "SELECT s.obj0.id, s.id FROM S3Object s", + "SELECT s.obj0.id, (s.id) FROM S3Object s", + "SELECT (s.obj0.id), s.obj2.\"s.obj0[id]\" FROM S3Object s", + "SELECT s.obj0.id AS id, s.a.id FROM S3Object s", + "SELECT s.obj0.id, s.x AS id FROM S3Object s", + "SELECT s.obj0.id AS \"s.a[id]\", s.b.id FROM S3Object s", + "SELECT s.obj.ID, s.id FROM S3Object s", + "SELECT s.obj.\"ID\", LOWER(s.id) FROM S3Object s", + "SELECT s.obj.id, s.obj0.id FROM S3Object s", + "SELECT s.a.id, s.b.id FROM S3Object s", + "SELECT s.a.id, s.b.id, s.obj2.obj0 FROM S3Object s", + "SELECT *, s.obj0.id FROM S3Object s", + "SELECT s.*, s.obj0.id FROM S3Object s", + "SELECT s.obj.\"lower(s.id)\", LOWER(s.id) FROM S3Object s", + "SELECT LOWER(s.id), s.obj.\"lower(s.id)\" FROM S3Object s", + "SELECT s.obj.\"count(*)\", COUNT(*) FROM S3Object s GROUP BY s.obj.\"count(*)\"", + "SELECT s.obj.\"s.n\", CAST(s.n AS INT) FROM S3Object s", + "SELECT s.obj.\"s.n + Int64(1)\", s.n + 1 FROM S3Object s", + "SELECT s.obj0.id, CAST(s.n AS INT) FROM S3Object s", + "SELECT s.obj0.id, s.obj1.id, s.obj2.\"s.obj0[id]\" FROM S3Object s", + "SELECT s.obj2.\"s.obj0[id]\", s.obj1.id, s.obj0.id FROM S3Object s", + "SELECT s.obj2.\"s.obj0[id]\", s.obj0.id FROM S3Object s", + "SELECT s.obj2.\"s.obj0[id]\", s.obj0.id, LOWER(s.id) FROM S3Object s", + "SELECT (s.obj0.id), (s.obj1.id) FROM S3Object s", + "SELECT s.obj.\"ID\", s.obj0.id FROM S3Object s", + ] { + match production_json_lines_query(BUCKET, KEY, sql).await { + Ok((names, _)) if names.iter().collect::>().len() != names.len() => { + failures.push(format!("{sql}: duplicate names {names:?}")); + } + Ok(_) => {} + Err(error) => failures.push(format!("{sql}: {error}")), + } + } + assert!(failures.is_empty(), "{failures:#?}"); + } + #[tokio::test] async fn csv_query_uses_custom_record_delimiter_across_file_partitions() { const ROW_COUNT: usize = 200_000; @@ -2841,4 +3183,110 @@ mod tests { assert_eq!(admission.available_permits(), 1); assert!(output.next().await.is_none()); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn path_alias_does_not_collide_with_computed_output_name() { + let mut input = (*json_snapshot_input()).clone(); + input.bucket = "s3select-review-name-collision".to_string(); + input.key = "collision.jsonl".to_string(); + input.request.expression = "SELECT s.obj.\"lower(s.id)\", LOWER(s.id) FROM S3Object s".to_string(); + let input = Arc::new(input); + let env = snapshot_test_env().await; + env.make_bucket(&input.bucket, false).await; + env.put_object_bytes(&input.bucket, &input.key, b"{\"id\":\"A\",\"obj\":{\"lower(s.id)\":7}}\n".to_vec()) + .await; + let snapshot = env.prepare_select_object_snapshot(&input.bucket, &input.key).await; + let dispatcher = production_dispatcher(Arc::clone(&input)); + let query = Query::new_with_snapshot( + QueryContext { + input: Arc::clone(&input), + }, + input.request.expression.clone(), + snapshot, + ); + let machine = dispatcher.build_query_state_machine(query).await.expect("production session"); + let result = dispatcher.build_logical_plan(Arc::clone(&machine)).await; + if let Err(error) = &result { + panic!( + "real production planner rejected a previously valid query: {error}; S3 classification={:?}", + error.s3_select_policy_error() + ); + } + let plan = result.expect("valid production query").expect("query plan"); + let Plan::Query(query_plan) = &plan; + let schema = query_plan.df_plan.schema(); + assert_eq!(schema.fields().len(), 2); + assert_ne!(schema.field(0).name(), schema.field(1).name()); + let output = dispatcher + .execute_logical_plan(plan, machine) + .await + .expect("execute production query"); + let Output::StreamData(stream) = output else { + panic!("query must return rows"); + }; + let batches: Vec<_> = stream.try_collect().await.expect("read production result"); + assert_eq!(batches.iter().map(|batch| batch.num_rows()).sum::(), 1); + let batch = batches.iter().find(|batch| batch.num_rows() == 1).expect("one result row"); + let key = batch + .column(0) + .as_any() + .downcast_ref::() + .expect("integer key"); + let lower = batch + .column(1) + .as_any() + .downcast_ref::() + .expect("lowercase string"); + assert_eq!(key.value(0), 7); + assert_eq!(lower.value(0), "a"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn path_alias_does_not_collide_with_computed_sort_name() { + let mut input = (*json_snapshot_input()).clone(); + input.bucket = "s3select-sort-name-collision".to_string(); + input.key = "collision.jsonl".to_string(); + input.request.expression = "SELECT s.obj.\"lower(s.id)\" FROM S3Object s ORDER BY LOWER(s.id)".to_string(); + let input = Arc::new(input); + let env = snapshot_test_env().await; + env.make_bucket(&input.bucket, false).await; + env.put_object_bytes(&input.bucket, &input.key, b"{\"id\":\"A\",\"obj\":{\"lower(s.id)\":7}}\n".to_vec()) + .await; + let snapshot = env.prepare_select_object_snapshot(&input.bucket, &input.key).await; + let dispatcher = production_dispatcher(Arc::clone(&input)); + let query = Query::new_with_snapshot( + QueryContext { + input: Arc::clone(&input), + }, + input.request.expression.clone(), + snapshot, + ); + let machine = dispatcher.build_query_state_machine(query).await.expect("production session"); + let result = dispatcher.build_logical_plan(Arc::clone(&machine)).await; + if let Err(error) = &result { + panic!( + "real production planner rejected a previously valid query: {error}; S3 classification={:?}", + error.s3_select_policy_error() + ); + } + let plan = result.expect("valid production query").expect("query plan"); + let Plan::Query(query_plan) = &plan; + let schema = query_plan.df_plan.schema(); + assert_eq!(schema.fields().len(), 1); + let output = dispatcher + .execute_logical_plan(plan, machine) + .await + .expect("execute production query"); + let Output::StreamData(stream) = output else { + panic!("query must return rows"); + }; + let batches: Vec<_> = stream.try_collect().await.expect("read production result"); + assert_eq!(batches.iter().map(|batch| batch.num_rows()).sum::(), 1); + let batch = batches.iter().find(|batch| batch.num_rows() == 1).expect("one result row"); + let key = batch + .column(0) + .as_any() + .downcast_ref::() + .expect("integer key"); + assert_eq!(key.value(0), 7); + } } diff --git a/crates/s3select-query/src/dispatcher/mod.rs b/crates/s3select-query/src/dispatcher/mod.rs index 4b9d89930..04121b55d 100644 --- a/crates/s3select-query/src/dispatcher/mod.rs +++ b/crates/s3select-query/src/dispatcher/mod.rs @@ -14,5 +14,6 @@ mod json_document_schema; mod json_document_table; +mod json_lines_schema; pub mod manager; mod parquet_table; diff --git a/crates/s3select-query/src/sql/planner.rs b/crates/s3select-query/src/sql/planner.rs index 57feec67c..cd0dbbc13 100644 --- a/crates/s3select-query/src/sql/planner.rs +++ b/crates/s3select-query/src/sql/planner.rs @@ -12,14 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{convert::Infallible, ops::ControlFlow}; +use std::{collections::HashMap, convert::Infallible, ops::ControlFlow}; use async_trait::async_trait; -use datafusion::sql::{ - planner::{IdentNormalizer, SqlToRel}, - sqlparser::ast::{ - AccessExpr, Expr, GroupByExpr, Ident, JsonPath, JsonPathElem, ObjectNamePart, OrderByKind, Query, Select, SelectFlavor, - SetExpr, Statement, Subscript, TableAlias, TableFactor, Value, Visit, VisitMut, Visitor, VisitorMut, +use datafusion::{ + common::{Column, Result as DFResult}, + logical_expr::{Expr as LogicalExpr, LogicalPlan, LogicalPlanBuilder}, + sql::{ + planner::{IdentNormalizer, SqlToRel}, + sqlparser::ast::{ + AccessExpr, Expr, GroupByExpr, Ident, JsonPath, JsonPathElem, ObjectNamePart, OrderBy, OrderByKind, Query, Select, + SelectFlavor, SelectItem, SetExpr, Statement, Subscript, TableAlias, TableFactor, Value, Visit, VisitMut, Visitor, + VisitorMut, + }, }, }; use rustfs_s3select_api::{ @@ -75,7 +80,9 @@ impl<'a, S: ContextProviderExtension + Send + Sync + 'a> SqlPlanner<'a, S> { } async fn df_prepared_sql_to_plan(&self, stmt: Statement, _session: &SessionCtx) -> QueryResult { + let keys = path_projection_keys(&stmt); let df_plan = self.df_planner.sql_statement_to_plan(stmt).map_err(classify_planner_error)?; + let df_plan = name_path_projections(df_plan, &keys)?; Ok(Plan::Query(QueryPlan { df_plan, is_tag_scan: false, @@ -150,9 +157,203 @@ pub(crate) fn prepare_s3_select_statement(statement: &mut Statement) -> QueryRes }; let mut normalizer = PartiQlSubscriptNormalizer; let _ = VisitMut::visit(query, &mut normalizer); + if let SetExpr::Select(select) = query.body.as_mut() { + alias_path_projections(select, query.order_by.as_ref()); + } Ok(source) } +/// Names an unaliased path projection after its last key, as AWS does +/// (`s.obj0.id` -> `id`), instead of DataFusion's `s.obj0[id]`. Names that +/// could collide with another output name keep DataFusion's name. Wildcard +/// columns, computed expressions, and retained paths have unknown output names +/// here, so they disable aliasing. Unprojected ORDER BY expressions also disable +/// aliasing because DataFusion adds their input columns to the projection. +/// GROUP BY can refer to these aliases; [`name_path_projections`] names the +/// paths left here once the planned output names are known. +fn alias_path_projections(select: &mut Select, order_by: Option<&OrderBy>) { + if has_wildcard(select) || sorts_by_unprojected_expr(select, order_by) { + return; + } + let Some(qualifier) = source_qualifier(select) else { + return; + }; + let normalizer = IdentNormalizer::default(); + let names: Vec> = select + .projection + .iter() + .map(|item| { + let ident = match item { + SelectItem::UnnamedExpr(Expr::Nested(_)) => None, + SelectItem::UnnamedExpr(expr) => column_name(expr), + SelectItem::ExprWithAlias { alias, .. } => Some(alias), + _ => None, + }; + ident.map(|ident| normalizer.normalize(ident.clone())) + }) + .collect(); + if names.iter().any(Option::is_none) { + return; + } + let mut counts = HashMap::::new(); + for name in names.iter().flatten() { + *counts.entry(name.clone()).or_default() += 1; + } + // Retained paths have generated names that cannot be inferred from their last key. + // Keep the whole projection unchanged so a sibling alias cannot collide with them. + if names.iter().flatten().any(|name| counts.get(name) != Some(&1)) { + return; + } + for item in &mut select.projection { + let SelectItem::UnnamedExpr(expr) = item else { + continue; + }; + let Some(alias) = path_key(expr, &qualifier).cloned() else { + continue; + }; + let expr = std::mem::replace(expr, Expr::Identifier(Ident::new(""))); + *item = SelectItem::ExprWithAlias { expr, alias }; + } +} + +/// The output name DataFusion gives a column reference: its last part. +fn column_name(expr: &Expr) -> Option<&Ident> { + match expr { + Expr::Identifier(ident) => Some(ident), + Expr::CompoundIdentifier(parts) => parts.last(), + Expr::Nested(inner) => column_name(inner), + _ => None, + } +} + +fn has_wildcard(select: &Select) -> bool { + select + .projection + .iter() + .any(|item| matches!(item, SelectItem::Wildcard(_) | SelectItem::QualifiedWildcard(..))) +} + +fn sorts_by_unprojected_expr(select: &Select, order_by: Option<&OrderBy>) -> bool { + let Some(OrderBy { + kind: OrderByKind::Expressions(sort_exprs), + .. + }) = order_by + else { + return false; + }; + sort_exprs.iter().any(|sort_expr| { + !select + .projection + .iter() + .any(|item| matches!(item, SelectItem::UnnamedExpr(projected) if *projected == sort_expr.expr)) + }) +} + +/// The normalized table alias, or table name, that may prefix a path. +fn source_qualifier(select: &Select) -> Option { + let normalizer = IdentNormalizer::default(); + match &select.from.first()?.relation { + TableFactor::Table { alias: Some(alias), .. } => Some(normalizer.normalize(alias.name.clone())), + TableFactor::Table { name, .. } => match name.0.first() { + Some(ObjectNamePart::Identifier(table_name)) => Some(normalizer.normalize(table_name.clone())), + _ => None, + }, + _ => None, + } +} + +/// The last key of a path of at least two keys after the source qualifier +/// (`s.obj0.id` -> `id`), the name AWS gives the projection. +fn path_key<'a>(expr: &'a Expr, qualifier: &str) -> Option<&'a Ident> { + let parts = match expr { + Expr::CompoundIdentifier(parts) => parts, + Expr::Nested(inner) => return path_key(inner, qualifier), + _ => return None, + }; + let path = match parts.split_first() { + Some((first, rest)) if IdentNormalizer::default().normalize(first.clone()) == qualifier => rest, + _ => parts.as_slice(), + }; + match path { + [_, _, ..] => path.last(), + _ => None, + } +} + +/// The normalized key of each path projection left unaliased by +/// [`alias_path_projections`]. Empty when a wildcard hides the output +/// positions, or when an ORDER BY expression is not projected. +fn path_projection_keys(statement: &Statement) -> Vec> { + let Statement::Query(query) = statement else { + return Vec::new(); + }; + let SetExpr::Select(select) = query.body.as_ref() else { + return Vec::new(); + }; + if has_wildcard(select) || sorts_by_unprojected_expr(select, query.order_by.as_ref()) { + return Vec::new(); + } + let Some(qualifier) = source_qualifier(select) else { + return Vec::new(); + }; + let normalizer = IdentNormalizer::default(); + select + .projection + .iter() + .map(|item| match item { + SelectItem::UnnamedExpr(expr) => path_key(expr, &qualifier).map(|key| normalizer.normalize(key.clone())), + _ => None, + }) + .collect() +} + +/// Renames each path projection to its key from [`path_projection_keys`] +/// unless another output column keeps or receives that name. Names come from +/// the planned schema, so computed, aliased and retained columns all count. +fn name_path_projections(plan: LogicalPlan, keys: &[Option]) -> DFResult { + let schema = plan.schema(); + if keys.len() != schema.fields().len() { + return Ok(plan); + } + let mut renames: Vec> = keys + .iter() + .zip(schema.fields()) + .map(|(key, field)| key.as_deref().filter(|key| key != field.name())) + .collect(); + // Dropping a rename brings back the generated name, which can collide with another rename. + loop { + let mut counts = HashMap::<&str, usize>::new(); + for (rename, field) in renames.iter().zip(schema.fields()) { + *counts.entry(rename.unwrap_or(field.name())).or_default() += 1; + } + let mut dropped = false; + for rename in &mut renames { + if rename.is_some_and(|name| counts[name] > 1) { + *rename = None; + dropped = true; + } + } + if !dropped { + break; + } + } + if renames.iter().all(Option::is_none) { + return Ok(plan); + } + let columns: Vec<_> = schema + .iter() + .zip(renames) + .map(|(column, rename)| { + let column = LogicalExpr::Column(Column::from(column)); + match rename { + Some(name) => column.alias(name), + None => column, + } + }) + .collect(); + LogicalPlanBuilder::from(plan).project(columns)?.build() +} + struct PartiQlSubscriptNormalizer; impl VisitorMut for PartiQlSubscriptNormalizer { @@ -374,7 +575,7 @@ impl Visitor for SubqueryDetector { #[cfg(test)] mod tests { - use super::prepare_s3_select_statement; + use super::{path_projection_keys, prepare_s3_select_statement}; use crate::sql::parser::ExtParser; use datafusion::sql::sqlparser::ast::{AccessExpr, Expr, Statement, Visit, Visitor}; use rustfs_s3select_api::{ @@ -596,6 +797,86 @@ mod tests { } } + fn prepared(sql: &str) -> String { + let mut statement = parse_statement(sql); + prepare_s3_select_statement(&mut statement).expect("statement should be supported"); + statement.to_string() + } + + #[test] + fn unaliased_path_projections_are_named_after_their_last_key() { + for (sql, expected) in [ + ("SELECT s.obj0.id FROM S3Object AS s", "SELECT s.obj0.id AS id FROM S3Object AS s"), + ( + "SELECT s.a.b.c, s.x.\"Y\" FROM S3Object AS s", + "SELECT s.a.b.c AS c, s.x.\"Y\" AS \"Y\" FROM S3Object AS s", + ), + ("SELECT obj0.id FROM S3Object", "SELECT obj0.id AS id FROM S3Object"), + ( + "SELECT s.obj0.id FROM S3Object AS s WHERE s.obj0.id = 'a' ORDER BY s.obj0.id", + "SELECT s.obj0.id AS id FROM S3Object AS s WHERE s.obj0.id = 'a' ORDER BY s.obj0.id", + ), + ] { + assert_eq!(prepared(sql), expected, "{sql}"); + } + } + + #[test] + fn projections_keep_their_names_when_a_key_alias_is_absent_or_ambiguous() { + for sql in [ + "SELECT s.id FROM S3Object AS s", + "SELECT S3Object.id FROM S3Object", + "SELECT s.tags[1] FROM S3Object AS s", + "SELECT s.obj0.id AS x FROM S3Object AS s", + "SELECT s.obj0.id, s.obj1.id FROM S3Object AS s", + "SELECT s.obj0.ID, s.obj1.id FROM S3Object AS s", + "SELECT s.obj0.id, s.id FROM S3Object AS s", + "SELECT s.obj0.id, s.name AS id FROM S3Object AS s", + "SELECT s.obj0.\"Id\", s.obj1.\"Id\" FROM S3Object AS s", + "SELECT *, s.obj0.id FROM S3Object AS s", + "SELECT s.*, s.obj0.id FROM S3Object AS s", + "SELECT s.obj0.id FROM S3Object AS s ORDER BY s.id", + "SELECT s.obj0.id FROM S3Object AS s ORDER BY upper(id)", + "SELECT s.obj0.id, (s.id) FROM S3Object AS s", + "SELECT s.obj.\"lower(s.id)\", LOWER(s.id) FROM S3Object AS s", + "SELECT s.obj.\"count(*)\", COUNT(*) FROM S3Object AS s GROUP BY s.obj.\"count(*)\"", + "SELECT s.obj0.id, COUNT(*) FROM S3Object AS s GROUP BY s.obj0.id", + ] { + assert_eq!(prepared(sql), sql); + } + } + + #[test] + fn path_projection_keys_cover_paths_left_unaliased() { + let keys = |sql: &str| { + let mut statement = parse_statement(sql); + prepare_s3_select_statement(&mut statement).expect("statement should be supported"); + path_projection_keys(&statement) + }; + let key = |key: &str| Some(key.to_owned()); + for (sql, expected) in [ + ("SELECT s.obj0.id FROM S3Object AS s", vec![None]), + ("SELECT s.obj0.id, s.obj1.id FROM S3Object AS s", vec![key("id"), key("id")]), + ( + "SELECT s.a.b.c, s.x.\"Y\", s.obj0.ID, lower(s.id) FROM S3Object AS s", + vec![key("c"), key("Y"), key("id"), None], + ), + ("SELECT obj0.id, COUNT(*) FROM S3Object GROUP BY obj0.id", vec![key("id"), None]), + ("SELECT (s.obj0.id), ((s.obj1.id)) FROM S3Object AS s", vec![key("id"), key("id")]), + ( + "SELECT s.id, s.tags[1], s.obj0.id AS x, (s.id) FROM S3Object AS s", + vec![None, None, None, None], + ), + ("SELECT S3Object.id, lower(id) FROM S3Object", vec![None, None]), + ("SELECT *, s.obj0.id FROM S3Object AS s", vec![]), + ("SELECT s.*, s.obj0.id FROM S3Object AS s", vec![]), + ("SELECT s.obj0.id FROM S3Object AS s ORDER BY s.id", vec![]), + ("SELECT s.obj0.id FROM S3Object AS s ORDER BY upper(id)", vec![]), + ] { + assert_eq!(keys(sql), expected, "{sql}"); + } + } + #[test] fn accepts_group_by_and_order_by() { let statement = parse_statement("SELECT department, COUNT(*) FROM S3Object GROUP BY department ORDER BY department"); diff --git a/docs/operations/audit-denial-entries.md b/docs/operations/audit-denial-entries.md new file mode 100644 index 000000000..d25ed7142 --- /dev/null +++ b/docs/operations/audit-denial-entries.md @@ -0,0 +1,39 @@ +# Audit Entries for Authorization Denials + +**Use this when:** you consume the audit stream and need to know which authorization denials appear in it, how many, and how to tune the limit. +**Source of truth:** `rustfs/src/storage/denial_audit.rs` (`DenialAuditor`, `DenialRateLimiter`), `rustfs/src/storage/access.rs` (`DenialContext`), `crates/config/src/constants/env.rs` (`ENV_AUDIT_DENIAL_RATE`, `ENV_AUDIT_DENIAL_ANONYMOUS_RATE`). + +With audit enabled (`RUSTFS_AUDIT_ENABLE=true`), a request refused by the authorization layer produces an audit entry in addition to the `s3_authorization_denied` log event. The S3 response is unchanged: a bare `Access Denied`. + +## Entry shape + +| Field | Value | +| --- | --- | +| `event` | `audit:S3:Operation` | +| `api.name` | the refused action, for example `s3:PutObject` | +| `api.bucket`, `api.object` | the request's bucket and object (the object is omitted for a bulk entry) | +| `api.status`, `api.statusCode` | `failure`, `403` | +| `accessKey` | the caller's access key, absent for an anonymous caller | +| `error` | `Access Denied` | +| `tags.denialReason` | the internal reason, for example `iam_explicit_deny`, `bucket_policy_explicit_deny`, `no_policy_allows_action` | +| `tags.deniedKeys` | bulk entries only: how many keys of a DeleteObjects request were refused for the action in `api.name` | +| `tags.suppressedDenials` | present when denials were dropped by the rate limit since the previous entry | + +## What is not audited + +- Probe denials that callers treat as an expected filter outcome (the per-bucket ListBuckets fallback and per-object metadata permission collection). +- A DeleteObjects request audits its refused keys as one entry per denied action (for example `s3:DeleteObject`, `s3:GetObject`, `s3:BypassGovernanceRetention`) with `tags.deniedKeys`, not one entry per key. +- Denials that are later overridden, such as an implicit IAM deny followed by a bucket policy allow. + +## Rate limit + +A caller can cause denials at will, so the number of entries is bounded by a process-wide token bucket per caller class. The burst equals the per-second rate. + +| Variable | Default | Meaning | +| --- | --- | --- | +| `RUSTFS_AUDIT_DENIAL_RATE` | `50` | Entries per second for authenticated callers. `0` turns denial entries off for them. | +| `RUSTFS_AUDIT_DENIAL_ANONYMOUS_RATE` | `5` | Entries per second for anonymous callers. `0` turns denial entries off for them. | + +Denials dropped by the limit are counted per caller class, and the count is reported in `tags.suppressedDenials` on the next entry of the same class that is let through. The limit is per process, not per node group. + +Trade-off: all authenticated callers share one bucket. A valid, low-privilege account that causes more denials per second than `RUSTFS_AUDIT_DENIAL_RATE` can use up the budget and crowd out other principals' denial entries during that time. Its own drops are still counted in `tags.suppressedDenials`, so the loss is visible, but the other principals' denials in that window are not recorded. A per-principal sub-limit would remove this and is a follow-up, not part of this change. diff --git a/rustfs/src/admin/handlers/object_zip_download.rs b/rustfs/src/admin/handlers/object_zip_download.rs index 43ee7495c..29eb4a16b 100644 --- a/rustfs/src/admin/handlers/object_zip_download.rs +++ b/rustfs/src/admin/handlers/object_zip_download.rs @@ -210,6 +210,7 @@ impl From for ReqInfo { region: current_region(), request_context: None, suppress_denial_log: false, + suppress_denial_audit: false, } } } @@ -1430,6 +1431,7 @@ mod tests { region: current_region(), request_context: None, suppress_denial_log: false, + suppress_denial_audit: false, } } diff --git a/rustfs/src/app/lifecycle_transition_api_test.rs b/rustfs/src/app/lifecycle_transition_api_test.rs index cd0d6f0b0..08cbbed25 100644 --- a/rustfs/src/app/lifecycle_transition_api_test.rs +++ b/rustfs/src/app/lifecycle_transition_api_test.rs @@ -2895,8 +2895,8 @@ async fn put_object_extract_computes_replication_decision_per_entry() { /// replication themselves. Without it a retention or legal hold applied after upload stays /// local and the replica keeps its previous, unprotected state (a WORM object that is still /// deletable on the peer). This observes the shared decision counter: each handler must -/// compute a replication decision exactly once, which is the step that drives both the -/// persisted pending marker and the schedule. +/// compute a replication decision exactly once when replication is configured, which is +/// the step that drives both the persisted pending marker and the schedule. #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[serial] #[ignore = "global-state usecase integration test: runs serialized in the CI ILM Integration (serial) lane, see ci.yml test-ilm-integration-serial and rustfs/backlog#1148 (ilm-1)"] @@ -2945,7 +2945,27 @@ async fn object_lock_handlers_schedule_replication() { retention change never replicates and the peer copy stays unprotected" ); - // PutObjectLegalHold must do the same. + // A bucket without a replication configuration can skip the decision. + MUST_REPLICATE_OBJECT_CALLS.store(0, Ordering::SeqCst); + let no_replication_input = PutObjectLegalHoldInput::builder() + .bucket(bucket.clone()) + .key(object.to_string()) + .legal_hold(Some(ObjectLockLegalHold { + status: Some(ObjectLockLegalHoldStatus::from_static(ObjectLockLegalHoldStatus::OFF)), + })) + .build() + .unwrap(); + fs.put_object_legal_hold(build_request(no_replication_input, Method::PUT)) + .await + .expect("legal hold without replication should succeed"); + assert_eq!( + MUST_REPLICATE_OBJECT_CALLS.load(Ordering::SeqCst), + 0, + "a bucket without replication must skip the decision" + ); + + // A configured bucket must still evaluate the metadata replication decision. + install_delete_replication_config(&bucket, true, false, None).await; MUST_REPLICATE_OBJECT_CALLS.store(0, Ordering::SeqCst); let legal_hold_input = PutObjectLegalHoldInput::builder() .bucket(bucket.clone()) diff --git a/rustfs/src/app/object/copy.rs b/rustfs/src/app/object/copy.rs index d1e25fabb..4e856830f 100644 --- a/rustfs/src/app/object/copy.rs +++ b/rustfs/src/app/object/copy.rs @@ -385,12 +385,6 @@ impl DefaultObjectUsecase { dst_opts.object_lock_config_snapshot = Some(store.object_lock_config_snapshot(&bucket).await.map_err(ApiError::from)?); } - let mut current_opts: ObjectOptions = internal_object_info_lookup_opts( - get_opts(&bucket, &key, dest_version_id.clone(), None, &req.headers) - .await - .map_err(ApiError::from)?, - ); - // Hold the self-copy namespace write lock before opening the source reader, including // expected-current historical copies. With lock optimization disabled, the source // stream retains its read guard until EOF; taking the write lock later in ECStore @@ -410,28 +404,42 @@ impl DefaultObjectUsecase { } dst_opts.expected_current_version_id = expected_current_version_id.clone(); - if _self_copy_lock_guard.is_some() { - current_opts.no_lock = true; - } - let previous_current_sizes = match store.get_object_info(&bucket, &key, ¤t_opts).await { - Ok(existing_obj_info) => { - validate_existing_object_lock_for_write(&object_lock_config_state, &existing_obj_info, &dst_opts)?; - if let Some(expected) = expected_current_version_id.as_deref() - && existing_obj_info.version_id.unwrap_or_default().to_string() != expected - { - return Err(s3_error!(PreconditionFailed)); - } - Some((existing_obj_info.size.max(0) as u64, quota_object_size(&existing_obj_info))) + let copy_prelookup_required = self_copy_lock + || expected_current_version_id.is_some() + || dest_version_id.is_some() + || (!super::shared::put_like_write_creates_new_version(&dst_opts) + && object_lock_checks_required_for_state(&object_lock_config_state)); + let previous_current_sizes: Option)>> = if copy_prelookup_required { + let mut current_opts: ObjectOptions = internal_object_info_lookup_opts( + get_opts(&bucket, &key, dest_version_id.clone(), None, &req.headers) + .await + .map_err(ApiError::from)?, + ); + if _self_copy_lock_guard.is_some() { + current_opts.no_lock = true; } - Err(err) => { - if expected_current_version_id.is_some() { - return Err(s3_error!(PreconditionFailed)); + Some(match store.get_object_info(&bucket, &key, ¤t_opts).await { + Ok(existing_obj_info) => { + validate_existing_object_lock_for_write(&object_lock_config_state, &existing_obj_info, &dst_opts)?; + if let Some(expected) = expected_current_version_id.as_deref() + && existing_obj_info.version_id.unwrap_or_default().to_string() != expected + { + return Err(s3_error!(PreconditionFailed)); + } + Some((existing_obj_info.size.max(0) as u64, quota_object_size(&existing_obj_info))) } - if !is_err_object_not_found(&err) && !is_err_version_not_found(&err) { - return Err(ApiError::from(err).into()); + Err(err) => { + if expected_current_version_id.is_some() { + return Err(s3_error!(PreconditionFailed)); + } + if !is_err_object_not_found(&err) && !is_err_version_not_found(&err) { + return Err(ApiError::from(err).into()); + } + None } - None - } + }) + } else { + None }; let (mut effective_sse, mut effective_kms_key_id) = resolve_bucket_default_sse( @@ -753,10 +761,11 @@ impl DefaultObjectUsecase { if let Some(quota_check) = quota_check.as_ref() { apply_quota_admission(&mut dst_opts, quota_check)?; } - let previous_current_size = match previous_current_sizes { - Some((_, Ok(logical_size))) if quota_enabled => Some(logical_size), - Some((_, Err(err))) if quota_enabled => return Err(ApiError::from(err).into()), - Some((physical_size, _)) => Some(physical_size), + let prelookup_previous_current_size: Option> = match previous_current_sizes { + Some(Some((_, Ok(logical_size)))) if quota_enabled => Some(Some(logical_size)), + Some(Some((_, Err(err)))) if quota_enabled => return Err(ApiError::from(err).into()), + Some(Some((physical_size, _))) => Some(Some(physical_size)), + Some(None) => Some(None), None => None, }; if let Some(quota_check) = quota_check.as_ref() { @@ -785,8 +794,8 @@ impl DefaultObjectUsecase { let _self_copy_lock_guard = _self_copy_lock_guard; let _table_data_plane_publication_guards = table_data_plane_publication_guards; - let oi = store - .copy_object(&src_bucket, &src_key, &bucket, &key, &mut src_info, &src_opts, &dst_opts) + let (oi, backfilled_old_current_size) = store + .copy_object_with_old_current_size(&src_bucket, &src_key, &bucket, &key, &mut src_info, &src_opts, &dst_opts) .await .map_err(ApiError::from)?; @@ -800,13 +809,22 @@ impl DefaultObjectUsecase { maybe_enqueue_transition_immediate(&oi, LcEventSrc::S3CopyObject).await; let _ = invalidate_object_data_cache_after_copy_success(&cache_adapter, &bucket, &key).await; - let dest_versioned = BucketVersioningSys::prefix_enabled(&bucket, &key).await; + let dest_versioned = dst_opts.versioned; if has_bucket_metadata { let committed_size = quota_accounting_object_size(&oi, quota_enabled)?; - if dest_versioned { - record_bucket_object_version_write_memory(&bucket, previous_current_size, committed_size).await; - } else { - record_bucket_object_write_memory(&bucket, previous_current_size, committed_size).await; + match prelookup_previous_current_size + .or_else(|| previous_current_size_from_backfill(backfilled_old_current_size)) + { + Some(previous_current_size) => { + if dest_versioned { + record_bucket_object_version_write_memory(&bucket, previous_current_size, committed_size).await; + } else { + record_bucket_object_write_memory(&bucket, previous_current_size, committed_size).await; + } + } + None => { + record_bucket_object_write_unknown_previous_memory(&bucket, committed_size, dest_versioned).await; + } } } diff --git a/rustfs/src/app/object/delete.rs b/rustfs/src/app/object/delete.rs index 28a7ccb58..018bfbbc0 100644 --- a/rustfs/src/app/object/delete.rs +++ b/rustfs/src/app/object/delete.rs @@ -568,6 +568,8 @@ impl DefaultObjectUsecase { // warn and demote the rest to debug, so a denied 1000-key DeleteObjects // cannot flood the log. let mut bulk_denial_logged = false; + // Per-key denials are audited once per denied action, in aggregate, after the loop. + let mut bulk_denials = crate::storage::access::BulkDenialAudit::start(&mut req)?; for (idx, obj_id) in delete.objects.iter().enumerate() { if let Err(err) = validate_object_key(&obj_id.key, "DeleteObjects") { delete_results[idx].error = Some(s3s::dto::Error { @@ -606,15 +608,18 @@ impl DefaultObjectUsecase { req_info.version_id = version_id.clone(); } - let mut auth_res = authorize_request(&mut req, delete_object_authorize_action(version_id.as_deref())).await; + let mut denied_action = delete_object_authorize_action(version_id.as_deref()); + let mut auth_res = authorize_request(&mut req, denied_action).await; if auth_res.is_ok() && delete_condition .as_ref() .is_some_and(DeleteObjectCondition::requires_read_permission) { - auth_res = authorize_request(&mut req, Action::S3Action(S3Action::GetObjectAction)).await; + denied_action = Action::S3Action(S3Action::GetObjectAction); + auth_res = authorize_request(&mut req, denied_action).await; } if auth_res.is_err() { + bulk_denials.denied(denied_action); if !bulk_denial_logged { bulk_denial_logged = true; req_info_mut(&mut req)?.suppress_denial_log = true; @@ -631,6 +636,7 @@ impl DefaultObjectUsecase { if bypass_governance { let auth_res = authorize_request(&mut req, Action::S3Action(S3Action::BypassGovernanceRetentionAction)).await; if auth_res.is_err() { + bulk_denials.denied(Action::S3Action(S3Action::BypassGovernanceRetentionAction)); if !bulk_denial_logged { bulk_denial_logged = true; req_info_mut(&mut req)?.suppress_denial_log = true; @@ -669,6 +675,8 @@ impl DefaultObjectUsecase { authorized_deletes.push(AuthorizedDelete { idx, object }); } + bulk_denials.finish(&mut req)?; + if authorized_deletes.is_empty() { let output = DeleteObjectsOutput { deleted: (!delete.quiet.unwrap_or_default()).then(Vec::new), @@ -1147,15 +1155,21 @@ impl DefaultObjectUsecase { let replicate_force_delete = force_delete && !replica && has_active_delete_rule(&delete_config_snapshot, &key); let mut force_delete_intent = None; - let existing_object_info = match store.get_object_info_for_delete(&bucket, &key, &opts).await { - Ok(obj_info) => Some(obj_info), - Err(err) => { - // If object not found, allow deletion to proceed (will return 204 No Content) - if !is_err_object_not_found(&err) && !is_err_version_not_found(&err) { - return Err(ApiError::from(err).into()); + // A version purge needs the original marker identity for replication. + let need_eager_pre_stat = opts.http_preconditions.is_some() || opts.version_id.is_some(); + let existing_object_info = if need_eager_pre_stat { + match Box::pin(store.get_object_info_for_delete(&bucket, &key, &opts)).await { + Ok(obj_info) => Some(obj_info), + Err(err) => { + // If object not found, allow deletion to proceed (will return 204 No Content) + if !is_err_object_not_found(&err) && !is_err_version_not_found(&err) { + return Err(ApiError::from(err).into()); + } + None } - None } + } else { + None }; #[cfg(test)] wait_for_delete_source_test_hook(&bucket).await; @@ -1209,16 +1223,21 @@ impl DefaultObjectUsecase { return result; } - if matches!(&err, StorageError::PrefixAccessDenied(_, _)) - && let Some(existing_object_info) = existing_object_info.as_ref() - && let Some(reason) = check_object_lock_for_deletion( - &bucket, - existing_object_info, - has_bypass_governance_header(&req.headers), - ) - .await - { - return Err(S3Error::with_message(S3ErrorCode::AccessDenied, reason.error_message())); + if matches!(&err, StorageError::PrefixAccessDenied(_, _)) { + let existing_object_info = match existing_object_info { + Some(info) => Some(info), + None => Box::pin(store.get_object_info_for_delete(&bucket, &key, &opts)).await.ok(), + }; + if let Some(existing_object_info) = existing_object_info.as_ref() + && let Some(reason) = check_object_lock_for_deletion( + &bucket, + existing_object_info, + has_bypass_governance_header(&req.headers), + ) + .await + { + return Err(S3Error::with_message(S3ErrorCode::AccessDenied, reason.error_message())); + } } return Err(ApiError::from(err).into()); @@ -3686,3 +3705,29 @@ mod tests { )); } } + +/// DeleteObjects refusals are audited once per denied action by `BulkDenialAudit`. The handler needs a +/// live store to run, so this source-level guard fails if the wiring is removed; it does not execute it. +#[cfg(test)] +mod bulk_denial_audit_wiring_tests { + #[test] + fn delete_objects_audits_refused_keys_in_aggregate() { + // Only the code above this module counts; the guard's own text must not satisfy it. + let source = include_str!("delete.rs") + .split("\n#[cfg(test)]\nmod bulk_denial_audit_wiring_tests") + .next() + .unwrap(); + let started = source + .find("BulkDenialAudit::start(&mut req)") + .expect("per-key denial audit is not suppressed"); + let finished = source + .find("bulk_denials.finish(&mut req)") + .expect("the aggregate entry is not emitted"); + assert!(started < finished, "the aggregate must be emitted after the per-key checks"); + assert_eq!( + source.matches("bulk_denials.denied(").count(), + 2, + "both authorization checks must count their denials" + ); + } +} diff --git a/rustfs/src/app/object/put.rs b/rustfs/src/app/object/put.rs index 0594d3b14..47cb20ff4 100644 --- a/rustfs/src/app/object/put.rs +++ b/rustfs/src/app/object/put.rs @@ -1734,7 +1734,9 @@ impl DefaultObjectUsecase { // replication), the lookup is skipped and accounting is backfilled from // the dst xl.meta that rename_data already reads, saving a full-disk // metadata fanout per PUT. - let prelookup_required = version_id.is_some() || object_lock_checks_required_for_state(&object_lock_config_state); + let prelookup_required = version_id.is_some() + || (!super::shared::put_like_write_creates_new_version(&opts) + && object_lock_checks_required_for_state(&object_lock_config_state)); #[cfg(test)] let prelookup_required = prelookup_required || injected_put_storage_failure(PutStorageTestStage::Prelookup).is_some(); // Outer None = prelookup skipped (accounting comes from the commit diff --git a/rustfs/src/app/object/shared.rs b/rustfs/src/app/object/shared.rs index 83e2c02ad..503989fa5 100644 --- a/rustfs/src/app/object/shared.rs +++ b/rustfs/src/app/object/shared.rs @@ -874,7 +874,7 @@ pub(crate) fn build_put_like_object_lock_metadata( Ok(Some(eval_metadata)) } -fn put_like_write_creates_new_version(opts: &ObjectOptions) -> bool { +pub(super) fn put_like_write_creates_new_version(opts: &ObjectOptions) -> bool { opts.version_id.is_none() && opts.versioned && !opts.version_suspended } diff --git a/rustfs/src/app/select_object.rs b/rustfs/src/app/select_object.rs index 9a51272f5..3ac691a26 100644 --- a/rustfs/src/app/select_object.rs +++ b/rustfs/src/app/select_object.rs @@ -600,9 +600,10 @@ fn validate_select_request(headers: &http::HeaderMap, input: &mut SelectObjectCo if input.request.expression.len() > MAX_SELECT_EXPRESSION_BYTES { return Err(S3Error::new(S3ErrorCode::ExpressionTooLong)); } - if input.request.expression_type.as_str() != ExpressionType::SQL { + let Some(expression_type) = canonical_enum(input.request.expression_type.as_str(), &[ExpressionType::SQL]) else { return Err(S3Error::new(S3ErrorCode::InvalidExpressionType)); - } + }; + input.request.expression_type = ExpressionType::from_static(expression_type); normalize_input_serialization(&mut input.request.input_serialization)?; let compressed_input = is_compressed_input(&input.request.input_serialization); @@ -649,6 +650,15 @@ fn is_compressed_input(input: &InputSerialization) -> bool { .is_some_and(|compression| compression.as_str() != CompressionType::NONE) } +/// Matches an S3 enum value in any ASCII case, as MinIO does, and returns the +/// canonical spelling that downstream code compares against. +fn canonical_enum(value: &str, allowed: &[&'static str]) -> Option<&'static str> { + allowed + .iter() + .copied() + .find(|candidate| candidate.eq_ignore_ascii_case(value)) +} + fn normalize_input_serialization(input: &mut InputSerialization) -> S3Result<()> { let format_count = usize::from(input.csv.is_some()) + usize::from(input.json.is_some()) + usize::from(input.parquet.is_some()); @@ -659,19 +669,20 @@ fn normalize_input_serialization(input: &mut InputSerialization) -> S3Result<()> return Err(S3Error::new(S3ErrorCode::ObjectSerializationConflict)); } - if let Some(compression) = input.compression_type.as_ref() { - match compression.as_str() { - CompressionType::NONE => {} - CompressionType::GZIP | CompressionType::BZIP2 => { - if input.parquet.is_some() { - return Err(S3Error::with_message( - S3ErrorCode::InvalidRequestParameter, - INVALID_REQUEST_PARAMETER_MESSAGE, - )); - } - } - _ => return Err(map_select_error_to_s3(&SelectError::InvalidCompressionFormat)), + if let Some(compression) = input.compression_type.as_mut() { + let Some(canonical) = canonical_enum( + compression.as_str(), + &[CompressionType::NONE, CompressionType::GZIP, CompressionType::BZIP2], + ) else { + return Err(map_select_error_to_s3(&SelectError::InvalidCompressionFormat)); + }; + if canonical != CompressionType::NONE && input.parquet.is_some() { + return Err(S3Error::with_message( + S3ErrorCode::InvalidRequestParameter, + INVALID_REQUEST_PARAMETER_MESSAGE, + )); } + *compression = CompressionType::from_static(canonical); } input .compression_type @@ -681,15 +692,16 @@ fn normalize_input_serialization(input: &mut InputSerialization) -> S3Result<()> let file_header_info = csv .file_header_info .get_or_insert_with(|| FileHeaderInfo::from_static(FileHeaderInfo::NONE)); - if !matches!( + let Some(canonical) = canonical_enum( file_header_info.as_str(), - FileHeaderInfo::NONE | FileHeaderInfo::USE | FileHeaderInfo::IGNORE - ) { + &[FileHeaderInfo::NONE, FileHeaderInfo::USE, FileHeaderInfo::IGNORE], + ) else { return Err(S3Error::with_message( S3ErrorCode::InvalidFileHeaderInfo, "The FileHeaderInfo value is not valid. Only NONE, USE, and IGNORE are supported.", )); - } + }; + *file_header_info = FileHeaderInfo::from_static(canonical); validate_single_byte(csv.comments.as_deref(), S3ErrorCode::InvalidRequestParameter)?; validate_single_character(csv.quote_character.as_deref())?; validate_single_character(csv.quote_escape_character.as_deref())?; @@ -699,9 +711,10 @@ fn normalize_input_serialization(input: &mut InputSerialization) -> S3Result<()> if let Some(json) = input.json.as_mut() { let json_type = json.type_.get_or_insert_with(|| JSONType::from_static(JSONType::LINES)); - if !matches!(json_type.as_str(), JSONType::DOCUMENT | JSONType::LINES) { + let Some(canonical) = canonical_enum(json_type.as_str(), &[JSONType::DOCUMENT, JSONType::LINES]) else { return Err(S3Error::new(S3ErrorCode::InvalidJsonType)); - } + }; + *json_type = JSONType::from_static(canonical); } Ok(()) @@ -716,15 +729,16 @@ fn normalize_output_serialization(output: &mut OutputSerialization) -> S3Result< return Err(S3Error::new(S3ErrorCode::ObjectSerializationConflict)); } - if let Some(csv) = output.csv.as_ref() { + if let Some(csv) = output.csv.as_mut() { validate_single_byte(csv.field_delimiter.as_deref(), S3ErrorCode::InvalidRequestParameter)?; validate_single_byte(csv.quote_character.as_deref(), S3ErrorCode::InvalidRequestParameter)?; validate_single_byte(csv.quote_escape_character.as_deref(), S3ErrorCode::InvalidRequestParameter)?; validate_output_record_delimiter(csv.record_delimiter.as_deref())?; - if let Some(quote_fields) = csv.quote_fields.as_ref() - && !matches!(quote_fields.as_str(), QuoteFields::ALWAYS | QuoteFields::ASNEEDED) - { - return Err(S3Error::new(S3ErrorCode::InvalidQuoteFields)); + if let Some(quote_fields) = csv.quote_fields.as_mut() { + let Some(canonical) = canonical_enum(quote_fields.as_str(), &[QuoteFields::ALWAYS, QuoteFields::ASNEEDED]) else { + return Err(S3Error::new(S3ErrorCode::InvalidQuoteFields)); + }; + *quote_fields = QuoteFields::from_static(canonical); } return Ok(SelectOutputFormat::Csv(csv.clone())); } @@ -3639,6 +3653,132 @@ mod tests { ); } + fn json_input(json_type: &str) -> SelectObjectContentInput { + let mut input = base_input(); + input.request.input_serialization.csv = None; + input.request.input_serialization.json = Some(JSONInput { + type_: Some(JSONType::from(json_type.to_string())), + }); + input + } + + #[test] + fn validate_matches_enum_values_case_insensitively_and_canonicalizes_them() { + for (value, expected) in [ + ("gzip", CompressionType::GZIP), + ("bZip2", CompressionType::BZIP2), + ("none", CompressionType::NONE), + ] { + let mut input = base_input(); + input.request.input_serialization.compression_type = Some(CompressionType::from(value.to_string())); + validate_select_request(&HeaderMap::new(), &mut input).expect("CompressionType should match any case"); + assert_eq!( + input + .request + .input_serialization + .compression_type + .as_ref() + .map(|v| v.as_str()), + Some(expected) + ); + } + + for (value, expected) in [("lines", JSONType::LINES), ("Document", JSONType::DOCUMENT)] { + let mut input = json_input(value); + validate_select_request(&HeaderMap::new(), &mut input).expect("JSON Type should match any case"); + let json_type = input + .request + .input_serialization + .json + .as_ref() + .and_then(|json| json.type_.as_ref()); + assert_eq!(json_type.map(|v| v.as_str()), Some(expected)); + } + let mut input = json_input("document"); + validate_select_request(&HeaderMap::new(), &mut input).expect("lowercase DOCUMENT should be accepted"); + assert!(is_json_document(input.request.input_serialization.json.as_ref().expect("JSON input"))); + + for (value, expected) in [ + ("use", FileHeaderInfo::USE), + ("Ignore", FileHeaderInfo::IGNORE), + ("none", FileHeaderInfo::NONE), + ] { + let mut input = base_input(); + input + .request + .input_serialization + .csv + .as_mut() + .expect("base input should use CSV") + .file_header_info = Some(FileHeaderInfo::from(value.to_string())); + validate_select_request(&HeaderMap::new(), &mut input).expect("FileHeaderInfo should match any case"); + let csv = input.request.input_serialization.csv.as_ref().expect("CSV input"); + assert_eq!(csv.file_header_info.as_ref().map(|v| v.as_str()), Some(expected)); + } + + for (value, expected) in [("always", QuoteFields::ALWAYS), ("AsNeeded", QuoteFields::ASNEEDED)] { + let mut input = base_input(); + input + .request + .output_serialization + .csv + .as_mut() + .expect("base output should use CSV") + .quote_fields = Some(QuoteFields::from(value.to_string())); + let validation = validate_select_request(&HeaderMap::new(), &mut input).expect("QuoteFields should match any case"); + let SelectOutputFormat::Csv(csv) = validation.output_format else { + panic!("CSV output expected"); + }; + assert_eq!(csv.quote_fields.as_ref().map(|v| v.as_str()), Some(expected)); + } + + let mut input = base_input(); + input.request.expression_type = ExpressionType::from("sql".to_string()); + validate_select_request(&HeaderMap::new(), &mut input).expect("ExpressionType should match any case"); + assert_eq!(input.request.expression_type.as_str(), ExpressionType::SQL); + } + + #[test] + fn validate_still_rejects_unknown_enum_values_in_any_case() { + for value in ["gzipx", "snappy", " GZIP"] { + let mut input = base_input(); + input.request.input_serialization.compression_type = Some(CompressionType::from(value.to_string())); + let error = validate_select_request(&HeaderMap::new(), &mut input).expect_err("unknown compression must fail"); + assert_eq!(error.code(), &S3ErrorCode::InvalidCompressionFormat, "{value}"); + } + for value in ["ndjson", "lines "] { + let error = + validate_select_request(&HeaderMap::new(), &mut json_input(value)).expect_err("unknown JSON type must fail"); + assert_eq!(error.code(), &S3ErrorCode::InvalidJsonType, "{value}"); + } + let mut input = base_input(); + input + .request + .input_serialization + .csv + .as_mut() + .expect("base input should use CSV") + .file_header_info = Some(FileHeaderInfo::from("used".to_string())); + let error = validate_select_request(&HeaderMap::new(), &mut input).expect_err("unknown header mode must fail"); + assert_eq!(error.code(), &S3ErrorCode::InvalidFileHeaderInfo); + + let mut input = base_input(); + input + .request + .output_serialization + .csv + .as_mut() + .expect("base output should use CSV") + .quote_fields = Some(QuoteFields::from("never".to_string())); + let error = validate_select_request(&HeaderMap::new(), &mut input).expect_err("unknown QuoteFields must fail"); + assert_eq!(error.code(), &S3ErrorCode::InvalidQuoteFields); + + let mut input = base_input(); + input.request.expression_type = ExpressionType::from("sqlite".to_string()); + let error = validate_select_request(&HeaderMap::new(), &mut input).expect_err("unknown ExpressionType must fail"); + assert_eq!(error.code(), &S3ErrorCode::InvalidExpressionType); + } + #[test] fn validate_accepts_two_byte_csv_input_record_delimiter() { let mut input = base_input(); diff --git a/rustfs/src/protocols/client.rs b/rustfs/src/protocols/client.rs index fbda71cb6..259b64aac 100644 --- a/rustfs/src/protocols/client.rs +++ b/rustfs/src/protocols/client.rs @@ -130,6 +130,7 @@ fn session_list_buckets_request( region: None, request_context: Some(RequestContext::fallback()), suppress_denial_log: false, + suppress_denial_audit: false, }); S3Request { @@ -324,6 +325,7 @@ impl ProtocolStorageClient { region: None, request_context: Some(RequestContext::fallback()), suppress_denial_log: false, + suppress_denial_audit: false, }); let req = S3Request { diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index c6c674a5d..daaf7f9e9 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -25,6 +25,8 @@ use crate::auth::{ use crate::error::{ApiError, normalize_read_quorum_error}; use crate::license::license_check; use crate::shared_types::RemoteAddr; +use crate::storage::denial_audit::DenialAuditor; +use crate::storage::helper::DenialScope; use crate::storage::request_context::RequestContext; use crate::storage::storage_api::contract::bucket::BUCKET_LIFECYCLE_LOCK_OBJECT; use crate::storage::storage_api::contract::namespace::NamespaceLocking as _; @@ -91,6 +93,9 @@ pub(crate) struct ReqInfo { /// collection) so `authorize_request` logs those routine denials at `debug` /// instead of `warn` (issue #5740). pub suppress_denial_log: bool, + /// Set by callers that audit their denials in aggregate (DeleteObjects), so each per-key + /// denial is not audited on its own. The denial is still logged. + pub suppress_denial_audit: bool, } pub(crate) fn replication_request_authorized(req: &S3Request) -> bool { @@ -1191,6 +1196,8 @@ pub(crate) fn log_list_buckets_iam_implicit_deny(req: &S3Request) -> S3Res let req_info = req_info_ref(req)?; let denial = DenialContext { quiet: true, + suppress_audit: true, + auditor: DenialAuditor::global(), bucket: req_info.bucket.as_deref().unwrap_or_default(), object: req_info.object.as_deref().unwrap_or_default(), version_id: req_info.version_id.as_deref(), @@ -1306,6 +1313,8 @@ pub(crate) fn owner_can_bypass_policy_deny(is_owner: bool, action: &Action) -> b /// instead of `warn`. struct DenialContext<'a> { quiet: bool, + suppress_audit: bool, + auditor: &'a DenialAuditor, bucket: &'a str, object: &'a str, version_id: Option<&'a str>, @@ -1342,14 +1351,68 @@ impl DenialContext<'_> { ); } - /// Build the generic AccessDenied response after recording the structured - /// denial event. - fn deny(&self, reason: &'static str, action: Action) -> S3Error { + /// Log the denial and, unless it is a quiet probe denial or audited in aggregate by the + /// caller, audit it. + fn record(&self, req: &S3Request, reason: &'static str, action: Action) { self.log(reason, action); + if !self.quiet && !self.suppress_audit { + self.auditor.audit(req, &action, reason, DenialScope::Request); + } + } + + /// Build the generic AccessDenied response after recording the structured + /// denial event and, unless this is a quiet probe denial, an audit entry. + fn deny(&self, req: &S3Request, reason: &'static str, action: Action) -> S3Error { + self.record(req, reason, action); s3_error!(AccessDenied, "Access Denied") } } +/// Audits a denial at an exit that has no [`DenialContext`] of its own. It only audits: those exits +/// keep their existing logging. The request's own probe flags are honored. +fn audit_request_denial(req: &S3Request, auditor: &DenialAuditor, reason: &'static str, action: Action) { + let info = req.extensions.get::(); + if info.is_some_and(|info| info.suppress_denial_log || info.suppress_denial_audit) { + return; + } + auditor.audit(req, &action, reason, DenialScope::Request); +} + +/// Audits the refused keys of a bulk request (DeleteObjects) in aggregate instead of once per key. +/// +/// `start` marks the request so per-key denials are logged but not audited; `finish` clears the +/// mark and emits one entry per denied action with the number of keys refused for it. +pub(crate) struct BulkDenialAudit { + denied: Vec<(Action, u64)>, +} + +impl BulkDenialAudit { + pub(crate) fn start(req: &mut S3Request) -> S3Result { + ext_req_info_mut(&mut req.extensions)?.suppress_denial_audit = true; + Ok(Self { denied: Vec::new() }) + } + + /// Counts one refused key for `action`. + pub(crate) fn denied(&mut self, action: Action) { + match self.denied.iter_mut().find(|(denied_action, _)| *denied_action == action) { + Some((_, count)) => *count += 1, + None => self.denied.push((action, 1)), + } + } + + pub(crate) fn finish(self, req: &mut S3Request) -> S3Result<()> { + self.finish_with(req, DenialAuditor::global()) + } + + fn finish_with(self, req: &mut S3Request, auditor: &DenialAuditor) -> S3Result<()> { + ext_req_info_mut(&mut req.extensions)?.suppress_denial_audit = false; + for (action, keys) in self.denied { + auditor.audit(req, &action, "bulk_request_keys_denied", DenialScope::Keys(keys)); + } + Ok(()) + } +} + /// Authorizes the request based on the action and credentials. pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3Result<()> { authorize_request_with_location_constraint(req, action, None).await @@ -1369,9 +1432,12 @@ async fn authorize_request_with_location_constraint( let object = req_info.object.clone().unwrap_or_default(); let version_id = req_info.version_id.clone(); let quiet_denial = req_info.suppress_denial_log; + let suppress_denial_audit = req_info.suppress_denial_audit; let store = request_object_store(req)?; let denial = DenialContext { quiet: quiet_denial, + suppress_audit: suppress_denial_audit, + auditor: DenialAuditor::global(), bucket: bucket.as_str(), object: object.as_str(), version_id: version_id.as_deref(), @@ -1472,7 +1538,7 @@ async fn authorize_request_with_location_constraint( .await .map_err(ApiError::from)? { - return Err(denial.deny("bucket_policy_explicit_deny", action)); + return Err(denial.deny(req, "bucket_policy_explicit_deny", action)); } let iam_allowed = { @@ -1514,7 +1580,7 @@ async fn authorize_request_with_location_constraint( .await .map_err(iam_authorization_error)? { - return Err(denial.deny("iam_explicit_deny", action)); + return Err(denial.deny(req, "iam_explicit_deny", action)); } } allowed @@ -1628,7 +1694,7 @@ async fn authorize_request_with_location_constraint( .await .map_err(ApiError::from)? { - return Err(denial.deny("bucket_policy_explicit_deny", action)); + return Err(denial.deny(req, "bucket_policy_explicit_deny", action)); } if action != Action::S3Action(S3Action::ListAllMyBucketsAction) { @@ -1670,11 +1736,11 @@ async fn authorize_request_with_location_constraint( deny_anonymous_table_data_plane_if_needed(req, action, bucket.as_str(), object.as_str()).await?; // RestrictPublicBuckets: when true, deny public access even if bucket policy allows it. match store.restricts_public_bucket_access(bucket_name).await { - Ok(true) => return Err(denial.deny("restrict_public_buckets", action)), + Ok(true) => return Err(denial.deny(req, "restrict_public_buckets", action)), Ok(false) => {} Err(StorageError::ConfigNotFound) => {} Err(_) => { - return Err(denial.deny("public_access_block_unavailable", action)); + return Err(denial.deny(req, "public_access_block_unavailable", action)); } } return Ok(()); @@ -1682,7 +1748,7 @@ async fn authorize_request_with_location_constraint( } } - Err(denial.deny("no_policy_allows_action", action)) + Err(denial.deny(req, "no_policy_allows_action", action)) } // Multipart parts are staged and become visible only after CompleteMultipartUpload, @@ -2178,6 +2244,7 @@ async fn authorize_table_data_plane_if_needed( ?admin_action, "table data-plane access denied by table resource policy" ); + audit_request_denial(req, DenialAuditor::global(), "table_resource_policy_denied", action); Err(s3_error!(AccessDenied, "Access Denied")) } @@ -2206,6 +2273,7 @@ async fn deny_anonymous_table_data_plane_if_needed( .await? .is_some() { + audit_request_denial(req, DenialAuditor::global(), "anonymous_table_data_plane_access", action); return Err(s3_error!(AccessDenied, "Access Denied")); } Ok(()) @@ -2642,6 +2710,12 @@ impl S3Access for FS { authorize_request(req, Action::S3Action(S3Action::ForceDeleteObjectAction)).await?; } if !recursive_force_delete_has_authenticated_caller(&req.headers, authenticated, replica_request) { + audit_request_denial( + req, + DenialAuditor::global(), + "recursive_force_delete_requires_authenticated_caller", + Action::S3Action(S3Action::ForceDeleteObjectAction), + ); return Err(s3_error!(AccessDenied, "Recursive force-delete requires an authenticated caller")); } @@ -3605,12 +3679,14 @@ mod tests { table_data_plane_content_mutation, table_data_plane_resource_for_request, table_publication_guard_error, validate_post_object_success_controls, versioned_read_action, versioned_tagging_action, }; + use super::{BulkDenialAudit, audit_request_denial}; use super::{ remaining_retention_days, requested_bucket_location_constraint, set_requested_object_lock_retain_until, set_requested_object_lock_retention, }; use crate::auth::OBJECT_LOCK_REMAINING_RETENTION_DAYS_CONDITION; use crate::error::ApiError; + use crate::storage::denial_audit::{DenialAuditor, DenialRateLimiter}; use crate::storage::storage_api::contract::bucket::{BucketOperations as _, DeleteBucketOptions, MakeBucketOptions}; use crate::storage::storage_api::contract::multipart::MultipartOperations as _; use crate::storage::storage_api::contract::object::{ObjectIO as _, ObjectOperations as _}; @@ -5107,21 +5183,268 @@ mod tests { )); } + type CapturedAudit = Arc>>; + + /// An auditor wired like the global one except that entries land in the returned vector. + fn test_auditor(enabled: bool, authenticated_rate: u64, anonymous_rate: u64) -> (DenialAuditor, CapturedAudit) { + let captured: CapturedAudit = Arc::default(); + let sink = Arc::clone(&captured); + // A frozen clock: no wall-clock refill can change what the limiter admits. + let t0 = std::time::Instant::now(); + let limiter = DenialRateLimiter::new(authenticated_rate, anonymous_rate, t0); + let auditor = + DenialAuditor::new(limiter, move || enabled, move |_, entry| sink.lock().unwrap().push(entry)).with_clock(move || t0); + (auditor, captured) + } + + fn denial_request(request_id: &str, authenticated: bool) -> S3Request<()> { + let mut req = build_request((), Method::GET); + req.uri = Uri::from_static("/audit-bucket/audit-object"); + req.extensions.insert(crate::storage::request_context::RequestContext { + request_id: request_id.to_string(), + x_amz_request_id: request_id.to_string(), + trace_id: None, + span_id: None, + start_time: std::time::Instant::now(), + }); + req.extensions.insert(ReqInfo { + cred: authenticated.then(|| rustfs_credentials::Credentials { + access_key: "denied-user".to_string(), + ..Default::default() + }), + bucket: Some("audit-bucket".to_string()), + object: Some("audit-object".to_string()), + ..Default::default() + }); + req + } + + fn denial_context(auditor: &DenialAuditor, quiet: bool) -> DenialContext<'_> { + DenialContext { + quiet, + suppress_audit: false, + auditor, + bucket: "audit-bucket", + object: "audit-object", + version_id: Some("version"), + account: Some("denied-user"), + is_owner: false, + } + } + + const GET_OBJECT: Action = Action::S3Action(S3Action::GetObjectAction); + /// Issue #5740: the denial helper must keep the client-facing error identical /// to the historical bare response — the added diagnostics are log-only. #[test] fn test_access_denied_helper_keeps_generic_wire_response() { - let denial = DenialContext { - quiet: false, - bucket: "bucket", - object: "object", - version_id: Some("version"), - account: Some("account"), - is_owner: false, - }; - let err = denial.deny("bucket_policy_explicit_deny", Action::S3Action(S3Action::GetObjectAction)); + let (auditor, captured) = test_auditor(true, 10, 10); + let req = denial_request("denial-wire-response", true); + let err = denial_context(&auditor, false).deny(&req, "bucket_policy_explicit_deny", GET_OBJECT); assert_eq!(err.code(), &S3ErrorCode::AccessDenied); assert_eq!(err.message(), Some("Access Denied")); + assert_eq!(captured.lock().unwrap().len(), 1); + } + + /// A real denial yields exactly one audit entry naming the account, bucket, object, + /// denied action and internal reason. + #[test] + fn denial_is_audited_once_with_account_resource_action_and_reason() { + let (auditor, captured) = test_auditor(true, 10, 10); + let req = denial_request("denial-audited-once", true); + let _ = denial_context(&auditor, false).deny(&req, "iam_explicit_deny", Action::S3Action(S3Action::PutObjectAction)); + + let entries = captured.lock().unwrap(); + assert_eq!(entries.len(), 1); + let entry = &entries[0]; + assert_eq!(entry.request_id.as_deref(), Some("denial-audited-once")); + assert_eq!(entry.access_key.as_deref(), Some("denied-user")); + assert_eq!(entry.api.name.as_deref(), Some("s3:PutObject")); + assert_eq!(entry.api.bucket.as_deref(), Some("audit-bucket")); + assert_eq!(entry.api.object.as_deref(), Some("audit-object")); + assert_eq!(entry.api.status.as_deref(), Some("failure")); + assert_eq!(entry.api.status_code, Some(403)); + assert_eq!(entry.error.as_deref(), Some("Access Denied")); + let tags = entry.tags.as_ref().expect("denial entry carries its reason"); + assert_eq!(tags.get("denialReason").and_then(|value| value.as_str()), Some("iam_explicit_deny")); + assert!(!tags.contains_key("suppressedDenials")); + } + + /// Probe denials (per-bucket ListBuckets fallback, per-object metadata collection) are + /// expected filter outcomes and must not flood the audit stream. + #[test] + fn quiet_probe_denial_is_not_audited() { + let (auditor, captured) = test_auditor(true, 10, 10); + let req = denial_request("denial-quiet-probe", true); + let err = + denial_context(&auditor, true).deny(&req, "no_policy_allows_action", Action::S3Action(S3Action::ListBucketAction)); + assert_eq!(err.code(), &S3ErrorCode::AccessDenied); + assert!(captured.lock().unwrap().is_empty()); + } + + #[test] + fn denial_is_not_audited_when_the_audit_module_is_off() { + let (auditor, captured) = test_auditor(false, 10, 10); + let req = denial_request("denial-audit-off", true); + let _ = denial_context(&auditor, false).deny(&req, "no_policy_allows_action", GET_OBJECT); + assert!(captured.lock().unwrap().is_empty()); + } + + #[test] + fn denial_audited_in_aggregate_by_the_caller_is_not_audited_per_key() { + let (auditor, captured) = test_auditor(true, 10, 10); + let req = denial_request("denial-per-key", true); + let mut denial = denial_context(&auditor, false); + denial.suppress_audit = true; + let _ = denial.deny(&req, "iam_explicit_deny", GET_OBJECT); + assert!(captured.lock().unwrap().is_empty()); + } + + /// A flood of denials emits a bounded number of entries; the rest are only counted. + #[test] + fn denial_flood_emits_bounded_entries() { + let (auditor, captured) = test_auditor(true, 3, 3); + let req = denial_request("denial-flood", true); + for _ in 0..20 { + let _ = denial_context(&auditor, false).deny(&req, "no_policy_allows_action", GET_OBJECT); + } + assert_eq!(captured.lock().unwrap().len(), 3); + } + + /// The entry that follows a suppressed run reports how many denials were dropped. + #[test] + fn entry_after_a_flood_reports_the_suppressed_count() { + use crate::storage::helper::DenialScope; + let (auditor, captured) = test_auditor(true, 1, 1); + let req = denial_request("denial-suppressed-count", true); + let t0 = std::time::Instant::now(); + for _ in 0..5 { + auditor.audit_at(&req, &GET_OBJECT, "no_policy_allows_action", DenialScope::Request, t0); + } + auditor.audit_at( + &req, + &GET_OBJECT, + "no_policy_allows_action", + DenialScope::Request, + t0 + std::time::Duration::from_secs(2), + ); + let entries = captured.lock().unwrap(); + assert_eq!(entries.len(), 2); + let tag = |index: usize| { + entries[index] + .tags + .as_ref() + .unwrap() + .get("suppressedDenials") + .and_then(|v| v.as_u64()) + }; + assert_eq!(tag(0), None); + assert_eq!(tag(1), Some(4)); + } + + /// Anonymous callers have their own, lower limit, so an unauthenticated scanner cannot + /// use up the budget for authenticated callers. + #[test] + fn anonymous_denials_are_limited_separately_from_authenticated_ones() { + let (auditor, captured) = test_auditor(true, 10, 1); + let anonymous = denial_request("denial-anonymous", false); + let authenticated = denial_request("denial-authenticated", true); + for _ in 0..5 { + let _ = denial_context(&auditor, false).deny(&anonymous, "no_policy_allows_action", GET_OBJECT); + } + for _ in 0..5 { + let _ = denial_context(&auditor, false).deny(&authenticated, "no_policy_allows_action", GET_OBJECT); + } + let entries = captured.lock().unwrap(); + let count = |id: &str| entries.iter().filter(|entry| entry.request_id.as_deref() == Some(id)).count(); + assert_eq!(count("denial-anonymous"), 1); + assert_eq!(count("denial-authenticated"), 5); + } + + /// Exits that have no `DenialContext` of their own audit through the request's own state + /// and honor the same probe flags. + #[test] + fn request_denial_exit_is_audited_and_honors_the_probe_flags() { + let (auditor, captured) = test_auditor(true, 10, 10); + let mut req = denial_request("denial-exit", true); + audit_request_denial(&req, &auditor, "table_resource_policy_denied", GET_OBJECT); + assert_eq!(captured.lock().unwrap().len(), 1); + + req.extensions.get_mut::().unwrap().suppress_denial_log = true; + audit_request_denial(&req, &auditor, "table_resource_policy_denied", GET_OBJECT); + assert_eq!(captured.lock().unwrap().len(), 1); + + req.extensions.get_mut::().unwrap().suppress_denial_log = false; + req.extensions.get_mut::().unwrap().suppress_denial_audit = true; + audit_request_denial(&req, &auditor, "table_resource_policy_denied", GET_OBJECT); + assert_eq!(captured.lock().unwrap().len(), 1); + } + + /// The table-policy, anonymous table and force-delete exits live deep in the authorization + /// flow and need a table catalog to reach. This source-level guard fails if one stops + /// auditing; it does not execute them. + #[test] + fn denial_exits_without_a_context_call_the_audit_function() { + // Only the production code counts; the tests' own calls must not satisfy the guard. + let source = include_str!("access.rs").split("\nmod tests {").next().unwrap(); + for reason in [ + "\"table_resource_policy_denied\"", + "\"anonymous_table_data_plane_access\"", + "\"recursive_force_delete_requires_authenticated_caller\"", + ] { + let call = source + .match_indices("audit_request_denial(") + .any(|(at, _)| source[at..].chars().take(260).collect::().contains(reason)); + assert!(call, "{reason} exit does not call audit_request_denial"); + } + } + + /// DeleteObjects audits its refused keys as one entry per denied action, carrying the count and + /// no single object, and the per-key mark is cleared afterwards. + #[test] + fn bulk_denial_is_one_entry_per_denied_action_with_the_key_count() { + let (auditor, captured) = test_auditor(true, 10, 10); + let mut req = denial_request("denial-bulk", true); + + let nothing_denied = BulkDenialAudit::start(&mut req).unwrap(); + assert!(req.extensions.get::().unwrap().suppress_denial_audit); + nothing_denied.finish_with(&mut req, &auditor).unwrap(); + assert!(!req.extensions.get::().unwrap().suppress_denial_audit); + assert!(captured.lock().unwrap().is_empty()); + + let mut bulk = BulkDenialAudit::start(&mut req).unwrap(); + let delete = Action::S3Action(S3Action::DeleteObjectAction); + let bypass = Action::S3Action(S3Action::BypassGovernanceRetentionAction); + for action in [delete, delete, bypass, delete] { + bulk.denied(action); + } + bulk.finish_with(&mut req, &auditor).unwrap(); + + let entries = captured.lock().unwrap(); + assert_eq!(entries.len(), 2); + let summary: Vec<_> = entries + .iter() + .map(|entry| { + assert_eq!(entry.api.object, None); + ( + entry.api.name.clone().unwrap(), + entry + .tags + .as_ref() + .unwrap() + .get("deniedKeys") + .and_then(|value| value.as_u64()) + .unwrap(), + ) + }) + .collect(); + assert_eq!( + summary, + vec![ + ("s3:DeleteObject".to_string(), 3), + ("s3:BypassGovernanceRetention".to_string(), 1) + ] + ); } #[test] diff --git a/rustfs/src/storage/denial_audit.rs b/rustfs/src/storage/denial_audit.rs new file mode 100644 index 000000000..ea4c6ce50 --- /dev/null +++ b/rustfs/src/storage/denial_audit.rs @@ -0,0 +1,254 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Audit entries for authorization denials. +//! +//! A denial is caused by the caller, not by an operation the server chose to run, so the +//! rate of entries is bounded: a process-wide token bucket per caller class (authenticated +//! and anonymous), with a lower default for anonymous callers. Suppressed denials are +//! counted and the count is carried on the next entry that is let through. + +use crate::module_switches::is_audit_module_enabled; +use crate::storage::access::{ReqInfo, request_context_from_req}; +use crate::storage::helper::{DenialRequest, DenialScope, authorization_denial_entry, spawn_background_with_context}; +use crate::storage::request_context::RequestContext; +use rustfs_audit::{AuditEntry, global::AuditLogger}; +use rustfs_policy::policy::action::Action; +use serde_json::Value; +use std::sync::{Mutex, OnceLock}; +use std::time::Instant; + +/// Tag on an entry that reports how many denials were suppressed since the previous entry. +pub(crate) const SUPPRESSED_DENIALS_TAG: &str = "suppressedDenials"; + +struct Bucket { + rate: f64, + burst: f64, + tokens: f64, + /// Denials dropped since this class last emitted an entry. + suppressed: u64, +} + +impl Bucket { + fn new(rate_per_sec: u64) -> Self { + let rate = rate_per_sec as f64; + let burst = rate.max(1.0); + Self { + rate, + burst, + tokens: burst, + suppressed: 0, + } + } + + fn refill(&mut self, elapsed_secs: f64) { + self.tokens = (self.tokens + elapsed_secs * self.rate).min(self.burst); + } + + /// Takes one token. `Suppress` drops the denial; a bucket with rate 0 is switched + /// off and does not count what it drops. + fn admit(&mut self) -> Admission { + if self.rate > 0.0 && self.tokens >= 1.0 { + self.tokens -= 1.0; + Admission::Emit { + suppressed: std::mem::take(&mut self.suppressed), + } + } else { + if self.rate > 0.0 { + self.suppressed = self.suppressed.saturating_add(1); + } + Admission::Suppress + } + } +} + +struct LimiterState { + authenticated: Bucket, + anonymous: Bucket, + last: Instant, +} + +/// Outcome of asking the limiter for one denial entry. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum Admission { + /// Emit the entry; `suppressed` denials were dropped since the previous one. + Emit { + suppressed: u64, + }, + Suppress, +} + +pub(crate) struct DenialRateLimiter { + state: Mutex, +} + +impl DenialRateLimiter { + pub(crate) fn new(authenticated_rate: u64, anonymous_rate: u64, now: Instant) -> Self { + Self { + state: Mutex::new(LimiterState { + authenticated: Bucket::new(authenticated_rate), + anonymous: Bucket::new(anonymous_rate), + last: now, + }), + } + } + + pub(crate) fn admit(&self, anonymous: bool, now: Instant) -> Admission { + let mut state = self.state.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + let elapsed = now.saturating_duration_since(state.last).as_secs_f64(); + state.last = now.max(state.last); + state.authenticated.refill(elapsed); + state.anonymous.refill(elapsed); + + if anonymous { + state.anonymous.admit() + } else { + state.authenticated.admit() + } + } +} + +type Emit = Box, AuditEntry) + Send + Sync>; + +/// Builds, bounds and dispatches denial audit entries. +pub(crate) struct DenialAuditor { + limiter: DenialRateLimiter, + enabled: Box bool + Send + Sync>, + emit: Emit, + clock: Box Instant + Send + Sync>, +} + +impl DenialAuditor { + pub(crate) fn new( + limiter: DenialRateLimiter, + enabled: impl Fn() -> bool + Send + Sync + 'static, + emit: impl Fn(Option, AuditEntry) + Send + Sync + 'static, + ) -> Self { + Self { + limiter, + enabled: Box::new(enabled), + emit: Box::new(emit), + clock: Box::new(Instant::now), + } + } + + /// Replaces the clock the limiter is driven by; tests use a frozen one so no wall-clock + /// refill can change an outcome. + #[cfg(test)] + pub(crate) fn with_clock(mut self, clock: impl Fn() -> Instant + Send + Sync + 'static) -> Self { + self.clock = Box::new(clock); + self + } + + /// The process-wide auditor: audit switch from the module state, rates from the environment, + /// entries dispatched to the audit system in the background. + pub(crate) fn global() -> &'static Self { + static AUDITOR: OnceLock = OnceLock::new(); + AUDITOR.get_or_init(|| { + let limiter = DenialRateLimiter::new( + rustfs_utils::get_env_u64(rustfs_config::ENV_AUDIT_DENIAL_RATE, rustfs_config::DEFAULT_AUDIT_DENIAL_RATE), + rustfs_utils::get_env_u64( + rustfs_config::ENV_AUDIT_DENIAL_ANONYMOUS_RATE, + rustfs_config::DEFAULT_AUDIT_DENIAL_ANONYMOUS_RATE, + ), + Instant::now(), + ); + DenialAuditor::new(limiter, is_audit_module_enabled, |context, entry| { + spawn_background_with_context(context, async move { + AuditLogger::log(entry).await; + }); + }) + }) + } + + pub(crate) fn audit(&self, req: &DenialRequest, action: &Action, reason: &'static str, scope: DenialScope) { + self.audit_at(req, action, reason, scope, (self.clock)()); + } + + pub(crate) fn audit_at( + &self, + req: &DenialRequest, + action: &Action, + reason: &'static str, + scope: DenialScope, + now: Instant, + ) { + if !(self.enabled)() { + return; + } + let anonymous = req.extensions.get::().is_none_or(|info| info.cred.is_none()); + let Admission::Emit { suppressed } = self.limiter.admit(anonymous, now) else { + return; + }; + let mut entry = authorization_denial_entry(req, action, reason, scope); + if suppressed > 0 { + entry + .tags + .get_or_insert_with(Default::default) + .insert(SUPPRESSED_DENIALS_TAG.to_string(), Value::from(suppressed)); + } + (self.emit)(request_context_from_req(req), entry); + } +} + +#[cfg(test)] +mod tests { + use super::{Admission, DenialRateLimiter}; + use std::time::{Duration, Instant}; + + #[test] + fn burst_is_bounded_and_suppressed_denials_are_counted_on_the_next_entry() { + let t0 = Instant::now(); + let limiter = DenialRateLimiter::new(2, 1, t0); + assert_eq!(limiter.admit(false, t0), Admission::Emit { suppressed: 0 }); + assert_eq!(limiter.admit(false, t0), Admission::Emit { suppressed: 0 }); + for _ in 0..5 { + assert_eq!(limiter.admit(false, t0), Admission::Suppress); + } + // One second later the bucket has refilled: the next entry reports what was dropped. + assert_eq!(limiter.admit(false, t0 + Duration::from_secs(1)), Admission::Emit { suppressed: 5 }); + assert_eq!(limiter.admit(false, t0 + Duration::from_secs(1)), Admission::Emit { suppressed: 0 }); + } + + #[test] + fn suppressed_counts_are_kept_per_caller_class() { + let t0 = Instant::now(); + let limiter = DenialRateLimiter::new(1, 1, t0); + assert_eq!(limiter.admit(true, t0), Admission::Emit { suppressed: 0 }); + for _ in 0..4 { + assert_eq!(limiter.admit(true, t0), Admission::Suppress); + } + // The authenticated class dropped nothing, so it must not inherit the anonymous drops. + assert_eq!(limiter.admit(false, t0), Admission::Emit { suppressed: 0 }); + assert_eq!(limiter.admit(true, t0 + Duration::from_secs(1)), Admission::Emit { suppressed: 4 }); + } + + #[test] + fn a_zero_rate_disables_that_class_without_counting() { + let t0 = Instant::now(); + let limiter = DenialRateLimiter::new(2, 0, t0); + assert_eq!(limiter.admit(true, t0), Admission::Suppress); + assert_eq!(limiter.admit(false, t0), Admission::Emit { suppressed: 0 }); + } + + #[test] + fn tokens_never_exceed_the_burst() { + let t0 = Instant::now(); + let limiter = DenialRateLimiter::new(2, 2, t0); + let later = t0 + Duration::from_secs(3600); + assert_eq!(limiter.admit(false, later), Admission::Emit { suppressed: 0 }); + assert_eq!(limiter.admit(false, later), Admission::Emit { suppressed: 0 }); + assert_eq!(limiter.admit(false, later), Admission::Suppress); + } +} diff --git a/rustfs/src/storage/ecfs.rs b/rustfs/src/storage/ecfs.rs index e58af776e..d0afd65c9 100644 --- a/rustfs/src/storage/ecfs.rs +++ b/rustfs/src/storage/ecfs.rs @@ -1815,12 +1815,13 @@ impl S3 for FS { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; - let _ = store - .get_bucket_info(&bucket, &BucketOptions::default()) - .await - .map_err(ApiError::from)?; - - validate_bucket_object_lock_enabled(&bucket).await?; + if let Err(err) = validate_bucket_object_lock_enabled(&bucket).await { + let _ = store + .get_bucket_info(&bucket, &BucketOptions::default()) + .await + .map_err(ApiError::from)?; + return Err(err); + } let opts: ObjectOptions = get_opts(&bucket, &key, version_id, None, &req.headers) .await @@ -1841,19 +1842,22 @@ impl S3 for FS { // once BEFORE the commit and persist the pending marker with it, so a restart between // the commit and the worker write-back leaves a Pending status on disk for the scanner // to re-drive. Scheduling without that marker would lose the task silently. - let dsc = match store.get_object_info(&bucket, &key, &opts).await.ok() { - Some(info) => { - must_replicate_metadata( - &bucket, - &key, - &info.user_defined, - info.user_tags.as_ref().clone(), - popts.delete_marker_replication_status(), - popts.clone(), - ) - .await - } - None => ReplicateDecision::new(), + let dsc = if crate::app::storage_api::bucket::metadata_sys::get_replication_config(&bucket) + .await + .is_ok() + && let Ok(info) = store.get_object_info(&bucket, &key, &opts).await + { + must_replicate_metadata( + &bucket, + &key, + &info.user_defined, + info.user_tags.as_ref().clone(), + popts.delete_marker_replication_status(), + popts.clone(), + ) + .await + } else { + ReplicateDecision::new() }; let mut eval_metadata = parse_object_lock_legal_hold(legal_hold)?; diff --git a/rustfs/src/storage/helper.rs b/rustfs/src/storage/helper.rs index f32203763..1a8b62601 100644 --- a/rustfs/src/storage/helper.rs +++ b/rustfs/src/storage/helper.rs @@ -22,12 +22,13 @@ use hashbrown::HashMap; use http::StatusCode; use metrics::counter; use rustfs_audit::{ - ObjectVersion, + ApiDetails, AuditEntry, ObjectVersion, entity::{ApiDetailsBuilder, AuditEntryBuilder}, global::AuditLogger, }; use rustfs_io_metrics::record_s3_op; use rustfs_notify::EventArgsBuilder; +use rustfs_policy::policy::action::Action; use rustfs_s3_ops::{S3Operation, operation_matches_event_name}; use rustfs_s3_types::EventName; use rustfs_targets::{ @@ -39,6 +40,9 @@ use s3s::{S3Request, S3Response, S3Result}; use serde_json::Value; use std::future::Future; use tokio::runtime::{Builder, Handle}; + +/// Request type seen by the denial auditor, so `denial_audit` stays off the direct s3s surface. +pub(crate) type DenialRequest = S3Request; use tracing::{Instrument, info_span, warn}; #[cfg(feature = "diagnostic-read-version-caller")] @@ -97,6 +101,42 @@ pub(crate) fn build_event_resp_elements(response: &S3Response, request_id: resp_elements } +/// Builds the audit entry shared by every request-scoped audit source: caller identity, +/// client address and the request line, without any outcome. +fn request_audit_builder( + req: &S3Request, + req_info: Option<&ReqInfo>, + event: EventName, + trigger: &str, + api: ApiDetails, + request_id: &str, +) -> AuditEntryBuilder { + let remote_host = req + .extensions + .get::() + .map(|info| info.real_ip.to_string()) + .or_else(|| { + req.extensions + .get::>() + .and_then(|addr| addr.map(|addr| addr.0.ip().to_string())) + }) + .unwrap_or_default(); + let mut builder = AuditEntryBuilder::new("1.0", event, trigger, api) + .remote_host(remote_host) + .user_agent(get_request_user_agent(&req.headers)) + .req_host(get_request_host(&req.headers)) + .req_path(req.uri.path().to_string()) + .req_query(extract_req_params(req)) + .request_id(request_id); + if let Some(cred) = req_info + .and_then(|info| info.cred.as_ref()) + .filter(|cred| !cred.access_key.is_empty()) + { + builder = builder.access_key(&cred.access_key); + } + builder +} + /// A unified helper structure for building and distributing audit logs and event notifications via RAII mode at the end of an S3 operation scope. pub enum OperationHelper { Disabled, @@ -205,34 +245,8 @@ impl OperationHelper { let request_context = request_context.unwrap_or_else(|| RequestContext::from_external_headers(&req.headers)); let request_id = request_context.request_id.clone(); - let audit_builder = if audit_enabled { - let remote_host = req - .extensions - .get::() - .map(|info| info.real_ip.to_string()) - .or_else(|| { - req.extensions - .get::>() - .and_then(|addr| addr.map(|addr| addr.0.ip().to_string())) - }) - .unwrap_or_default(); - let mut builder = AuditEntryBuilder::new("1.0", event, trigger, api_builder.clone().build()) - .remote_host(remote_host) - .user_agent(get_request_user_agent(&req.headers)) - .req_host(get_request_host(&req.headers)) - .req_path(req.uri.path().to_string()) - .req_query(extract_req_params(req)) - .request_id(&request_id); - if let Some(cred) = req_info - .and_then(|info| info.cred.as_ref()) - .filter(|cred| !cred.access_key.is_empty()) - { - builder = builder.access_key(&cred.access_key); - } - Some(builder) - } else { - None - }; + let audit_builder = + audit_enabled.then(|| request_audit_builder(req, req_info, event, trigger, api_builder.clone().build(), &request_id)); let event_object = ObjectInfo { bucket: bucket.clone(), @@ -440,6 +454,73 @@ impl OperationHelper { } } +/// What an authorization denial entry describes. +#[derive(Clone, Copy, Debug)] +pub(crate) enum DenialScope { + /// One refused request for the request's own bucket and object. + Request, + /// A bulk request (DeleteObjects) in which this many keys were refused; no single object applies. + Keys(u64), +} + +/// Builds the audit entry for a request the authorization layer refused. +/// +/// The entry carries the action that was denied and the internal denial reason, so an +/// audit consumer can tell a refused request from a handler that never ran. +pub(crate) fn authorization_denial_entry( + req: &S3Request, + action: &Action, + reason: &'static str, + scope: DenialScope, +) -> AuditEntry { + let req_info = req.extensions.get::(); + let path = req.uri.path().trim_start_matches('/'); + let mut segs = path.splitn(2, '/'); + let path_bucket = segs.next().unwrap_or(""); + let path_object = segs.next().unwrap_or(""); + let bucket = req_info + .and_then(|info| info.bucket.as_deref()) + .filter(|value| !value.is_empty()) + .unwrap_or(path_bucket); + let object = match scope { + DenialScope::Request => req_info + .and_then(|info| info.object.as_deref()) + .filter(|value| !value.is_empty()) + .unwrap_or(path_object), + DenialScope::Keys(_) => "", + }; + + let action_name: &str = action.into(); + let mut api = ApiDetailsBuilder::new() + .name(action_name) + .status("failure") + .status_code(StatusCode::FORBIDDEN.as_u16() as i32); + if !bucket.is_empty() { + api = api.bucket(bucket); + } + if !object.is_empty() { + api = api.object(object); + } + + let request_context = request_context_from_req(req).unwrap_or_else(|| RequestContext::from_external_headers(&req.headers)); + let mut tags = HashMap::new(); + tags.insert("denialReason".to_string(), Value::String(reason.to_string())); + if let DenialScope::Keys(denied) = scope { + tags.insert("deniedKeys".to_string(), Value::from(denied)); + } + request_audit_builder( + req, + req_info, + EventName::AuditS3Operation, + action_name, + api.build(), + &request_context.request_id, + ) + .error("Access Denied") + .tags(tags) + .build() +} + /// Runs `handler` on `req` and records the outcome as an audit-only entry for `op`. /// /// The helper is built before the request is handed over so it sees the original headers diff --git a/rustfs/src/storage/mod.rs b/rustfs/src/storage/mod.rs index 10630bb01..d36e72b79 100644 --- a/rustfs/src/storage/mod.rs +++ b/rustfs/src/storage/mod.rs @@ -15,6 +15,7 @@ pub mod access; pub mod concurrency; pub mod deadlock_detector; +pub(crate) mod denial_audit; pub mod ecfs; pub(crate) mod helper; pub mod options; diff --git a/scripts/ci_test_shards.py b/scripts/ci_test_shards.py index c3f2d7900..112593d76 100644 --- a/scripts/ci_test_shards.py +++ b/scripts/ci_test_shards.py @@ -73,7 +73,7 @@ def partition(listing, junit, protected, count=2): durations[key] = seconds require(set(durations) == expected, "duration baseline does not cover the selected tests") binaries = {binary for binary, _ in expected} - require(all(re.fullmatch(r"[A-Za-z0-9_.:-]+", binary) for binary in binaries), "unsupported binary ID") + require(all(re.fullmatch(r"[A-Za-z0-9_./:-]+", binary) for binary in binaries), "unsupported binary ID") reserved = {binary for binary, _ in protected} components = [reserved] if reserved else [] components += [{binary} for binary in sorted(binaries - reserved)] @@ -130,7 +130,7 @@ def load_plan(directory): protected = set(plan["protected_binaries"]) require(protected <= expected and any(protected <= set(shard["binaries"]) for shard in shards), "split resource component") for shard in shards: - require(shard["binaries"] and all(re.fullmatch(r"[A-Za-z0-9_.:-]+", name) for name in shard["binaries"]), "invalid shard binary") + require(shard["binaries"] and all(re.fullmatch(r"[A-Za-z0-9_./:-]+", name) for name in shard["binaries"]), "invalid shard binary") require(shard["filter"] == " or ".join(f"binary_id(={name})" for name in shard["binaries"]), "shard filter mismatch") return plan, listing diff --git a/scripts/test_ci_test_shards.py b/scripts/test_ci_test_shards.py index 8c0fb2f09..bddcc5a22 100644 --- a/scripts/test_ci_test_shards.py +++ b/scripts/test_ci_test_shards.py @@ -77,6 +77,7 @@ class RealNextestTests(unittest.TestCase): (root / 'rust-toolchain.toml').write_text((shards.ROOT / 'rust-toolchain.toml').read_text()) (root / 'Cargo.toml').write_text('[package]\nname="probe"\nversion="0.1.0"\nedition="2021"\n') (root / 'src/lib.rs').write_text('#[test] fn first() { assert!(std::env::var_os("SHARD_FAIL").is_none()); }\n') + (root / 'src/main.rs').write_text('fn main() {}\n#[test] fn binary_case() {}\n') (root / 'tests/second.rs').write_text('#[test] fn second() {}\n#[test] #[ignore] fn omitted() {}\n') (root / 'tests/third.rs').write_text('#[test] fn third() {}\n') (root / '.config/nextest.toml').write_text('''experimental = ["setup-scripts"] @@ -103,6 +104,7 @@ setup = ['fixture'] return result cargo('archive', '--offline', '--archive-file', str(archive)) full = shards.list_tests(root, ['--archive-file', str(archive), '--extract-to', str(root), '--extract-overwrite']) + self.assertIn('probe::bin/probe', full['rust-suites']) full_path = root / 'listing.json' shards.write_json(full_path, full) cargo('run', '--profile', 'ci', *shards.metadata_arguments(root)) @@ -118,7 +120,7 @@ setup = ['fixture'] evidence = root / 'evidence' for shard in plan['shards']: shards.execute(root, plan_dir, archive, shard['index'], evidence / str(shard['index'])) - self.assertEqual(shards.verify(plan_dir, evidence), 3) + self.assertEqual(shards.verify(plan_dir, evidence), 4) floor = root / 'migration-floor.txt' floor.write_text('1\n') # Older local nextest versions omit the test kind; all fixture