fix(cache): harden object data cache coordination (#5004)

* fix(cache): enforce projected entry capacity Refs: rustfs/backlog#1335 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): fence identity budget eviction by generation Refs rustfs/backlog#1334. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): fence clear against concurrent fills Refs rustfs/backlog#1333 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): linearize memory reservation claims Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): retain allocation memory claims Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): publish memory snapshots by epoch Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): coordinate cold object fills Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): fence metadata cache transition races Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com>
2026-08-21 11:56:38 +00:00 · 2026-07-18 22:37:10 +08:00
parent 4faea7fcbc
commit 15f4e75870
39 changed files with 9930 additions and 538 deletions
@@ -21,8 +21,9 @@

 use super::super::*;

+use crate::bucket::lifecycle::tier_sweeper::delete_object_from_remote_tier_idempotent;
 use crate::disk::OldCurrentSize;
-use crate::object_api::GetObjectBodySource;
+use crate::object_api::{GetObjectBodySource, get_object_body_cache_hook_suppressed};

 /// Length of the full plaintext body when — and only when — this read's output
 /// is exactly the object's complete plaintext, so the app-layer body cache may
@@ -61,6 +62,11 @@ fn full_object_plaintext_len(range: &Option<HTTPRangeSpec>, opts: &ObjectOptions
        || crate::object_api::restore_request_active(opts)
        || object_info.is_encrypted()
        || object_info.is_remote()
+        || object_info.delete_marker
+        || object_info.size == 0
+        || object_info.version_only
+        || object_info.metadata_only
+        || object_info.is_inline_fast_path_eligible()
    {
        return None;
    }
@@ -72,6 +78,14 @@ fn full_object_plaintext_len(range: &Option<HTTPRangeSpec>, opts: &ObjectOptions
    Some(object_info.size)
 }

+pub(crate) fn body_cache_plaintext_len(
+    range: &Option<HTTPRangeSpec>,
+    opts: &ObjectOptions,
+    object_info: &ObjectInfo,
+) -> Option<i64> {
+    full_object_plaintext_len(range, opts, object_info)
+}
+
 #[async_trait::async_trait]
 impl crate::storage_api_contracts::object::ObjectIO for SetDisks {
    type Error = Error;
@@ -131,16 +145,21 @@ impl crate::storage_api_contracts::object::ObjectIO for SetDisks {
        };

        let metadata_stage_start = Instant::now();
-        let (fi, files, disks) = match self.get_object_fileinfo(bucket, object, opts, true, true).await {
-            Ok(result) => result,
-            Err(err) => {
-                rustfs_io_metrics::record_get_object_metadata_phase_duration(metadata_stage_start.elapsed().as_secs_f64());
-                record_get_object_pipeline_failure(GET_STAGE_METADATA, classify_storage_error(&err));
-                return Err(to_object_err(err, vec![bucket, object]));
+        let (fi, files, disks, prepared_object_info) = if let Some(prepared) = take_prepared_get_object_metadata() {
+            (prepared.fi, prepared.files, prepared.disks, prepared.object_info)
+        } else {
+            match self.get_object_fileinfo(bucket, object, opts, true, true).await {
+                Ok((fi, files, disks)) => (fi, files, disks, None),
+                Err(err) => {
+                    rustfs_io_metrics::record_get_object_metadata_phase_duration(metadata_stage_start.elapsed().as_secs_f64());
+                    record_get_object_pipeline_failure(GET_STAGE_METADATA, classify_storage_error(&err));
+                    return Err(to_object_err(err, vec![bucket, object]));
+                }
            }
        };
        let object_info_stage_start = get_stage_timer_if_enabled(stage_metrics_enabled);
-        let object_info = ObjectInfo::from_file_info(&fi, bucket, object, opts.versioned || opts.version_suspended);
+        let object_info = prepared_object_info
+            .unwrap_or_else(|| build_get_object_info(&fi, bucket, object, opts.versioned || opts.version_suspended));
        let object_class = classify_get_codec_streaming_object_class(&range, &object_info, &fi);
        let size_bucket = rustfs_io_metrics::get_object_size_bucket(object_info.size);
        record_get_stage_duration_if_enabled(GET_OBJECT_PATH_SET_DISK, GET_STAGE_OBJECT_INFO, object_info_stage_start);
@@ -410,6 +429,7 @@ impl crate::storage_api_contracts::object::ObjectIO for SetDisks {
        // it forward.
        let mut body_source = GetObjectBodySource::Unprobed;
        if let Some(plaintext_len) = full_object_plaintext_len(&range, opts, &object_info)
+            && !get_object_body_cache_hook_suppressed()
            && let Some(hook) = get_object_body_cache_hook()
        {
            match hook.lookup(bucket, object, &object_info).await {
@@ -1222,6 +1242,178 @@ impl SetDisks {
    }
 }

+async fn cleanup_uncommitted_transition_upload(tier: &str, object: &str, remote_version: &str) {
+    if let Err(err) = delete_object_from_remote_tier_idempotent(object, remote_version, tier).await {
+        warn!(
+            tier,
+            object,
+            remote_version,
+            error = ?err,
+            "failed to clean uncommitted transition upload"
+        );
+    }
+}
+
+struct TransitionUploadCleanup {
+    tier: String,
+    object: String,
+    remote_version: String,
+    armed: bool,
+}
+
+impl TransitionUploadCleanup {
+    fn new(tier: &str, object: &str, remote_version: &str) -> Self {
+        Self {
+            tier: tier.to_string(),
+            object: object.to_string(),
+            remote_version: remote_version.to_string(),
+            armed: true,
+        }
+    }
+
+    async fn cleanup(&mut self) {
+        cleanup_uncommitted_transition_upload(&self.tier, &self.object, &self.remote_version).await;
+        self.armed = false;
+    }
+
+    fn disarm(&mut self) {
+        self.armed = false;
+    }
+}
+
+impl Drop for TransitionUploadCleanup {
+    fn drop(&mut self) {
+        if !self.armed {
+            return;
+        }
+        let tier = self.tier.clone();
+        let object = self.object.clone();
+        let remote_version = self.remote_version.clone();
+        if let Ok(handle) = tokio::runtime::Handle::try_current() {
+            handle.spawn(async move {
+                cleanup_uncommitted_transition_upload(&tier, &object, &remote_version).await;
+            });
+        }
+    }
+}
+
+#[cfg(test)]
+struct TransitionCommitBarrierState {
+    bucket: String,
+    object: String,
+    arrived: tokio::sync::Notify,
+    release: tokio::sync::Notify,
+}
+
+#[cfg(test)]
+struct TransitionCommitBarrier {
+    state: Arc<TransitionCommitBarrierState>,
+}
+
+#[cfg(test)]
+static TRANSITION_COMMIT_BARRIER: std::sync::OnceLock<std::sync::Mutex<Option<Arc<TransitionCommitBarrierState>>>> =
+    std::sync::OnceLock::new();
+
+#[cfg(test)]
+impl TransitionCommitBarrier {
+    fn install(bucket: &str, object: &str) -> Self {
+        let state = Arc::new(TransitionCommitBarrierState {
+            bucket: bucket.to_string(),
+            object: object.to_string(),
+            arrived: tokio::sync::Notify::new(),
+            release: tokio::sync::Notify::new(),
+        });
+        let mut slot = TRANSITION_COMMIT_BARRIER
+            .get_or_init(|| std::sync::Mutex::new(None))
+            .lock()
+            .expect("transition commit barrier mutex should not poison");
+        assert!(slot.is_none(), "transition commit barrier must be installed by one test at a time");
+        *slot = Some(Arc::clone(&state));
+        drop(slot);
+        Self { state }
+    }
+
+    async fn wait_until_paused(&self) {
+        tokio::time::timeout(Duration::from_secs(30), self.state.arrived.notified())
+            .await
+            .expect("transition should reach the deterministic commit barrier");
+    }
+
+    fn release(&self) {
+        self.state.release.notify_one();
+    }
+}
+
+#[cfg(test)]
+impl Drop for TransitionCommitBarrier {
+    fn drop(&mut self) {
+        self.state.release.notify_one();
+        let mut slot = TRANSITION_COMMIT_BARRIER
+            .get_or_init(|| std::sync::Mutex::new(None))
+            .lock()
+            .expect("transition commit barrier mutex should not poison");
+        if slot.as_ref().is_some_and(|state| Arc::ptr_eq(state, &self.state)) {
+            *slot = None;
+        }
+    }
+}
+
+#[cfg(test)]
+async fn pause_transition_before_local_commit(bucket: &str, object: &str) {
+    let barrier = TRANSITION_COMMIT_BARRIER
+        .get_or_init(|| std::sync::Mutex::new(None))
+        .lock()
+        .expect("transition commit barrier mutex should not poison")
+        .as_ref()
+        .filter(|barrier| barrier.bucket == bucket && barrier.object == object)
+        .cloned();
+    if let Some(barrier) = barrier {
+        barrier.arrived.notify_one();
+        barrier.release.notified().await;
+    }
+}
+
+fn parse_transition_version_id(remote_version: &str) -> std::result::Result<Option<Uuid>, uuid::Error> {
+    if remote_version.is_empty() {
+        return Ok(None);
+    }
+    Uuid::parse_str(remote_version).map(|version_id| (!version_id.is_nil()).then_some(version_id))
+}
+
+fn transition_cleanup_remote_version(remote_version: &str, version_id: Option<Uuid>) -> &str {
+    version_id.map(|_| remote_version).unwrap_or("")
+}
+
+#[cfg(test)]
+mod transition_version_id_tests {
+    use super::{parse_transition_version_id, transition_cleanup_remote_version};
+    use uuid::Uuid;
+
+    #[test]
+    fn normalizes_unversioned_remote_ids() {
+        assert_eq!(parse_transition_version_id("").expect("empty remote version should be valid"), None);
+        assert_eq!(
+            parse_transition_version_id(&Uuid::nil().to_string()).expect("nil remote version should be valid"),
+            None
+        );
+        assert_eq!(transition_cleanup_remote_version(&Uuid::nil().to_string(), None), "");
+    }
+
+    #[test]
+    fn preserves_valid_remote_id_and_rejects_invalid_text() {
+        let version_id = Uuid::new_v4();
+        assert_eq!(
+            parse_transition_version_id(&version_id.to_string()).expect("UUID remote version should be valid"),
+            Some(version_id)
+        );
+        assert_eq!(
+            transition_cleanup_remote_version(&version_id.to_string(), Some(version_id)),
+            version_id.to_string()
+        );
+        assert!(parse_transition_version_id("not-a-uuid").is_err());
+    }
+}
+
 #[async_trait::async_trait]
 impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
    type Error = Error;
@@ -1908,7 +2100,7 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
                .await
                .map_err(|e| to_object_err(e.into(), vec![bucket, object]))?;

-            self.get_object_metadata_cache.invalidate_all();
+            self.invalidate_all_get_object_metadata_cache();
            return Ok(ObjectInfo::default());
        }

@@ -2236,7 +2428,7 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
        if let Some(mod_time1) = opts.mod_time {
            if let Some(mod_time2) = fi.mod_time.as_ref() {
                if mod_time1.unix_timestamp() != mod_time2.unix_timestamp()
-                /*|| transition_etag != stored_etag*/
+                    || (!transition_etag.is_empty() && transition_etag != stored_etag)
                {
                    return Err(to_object_err(Error::other(DiskError::FileNotFound), vec![bucket, object]));
                }
@@ -2333,25 +2525,88 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
            return Err(StorageError::Io(err));
        }
        let rv = rv?;
-        fi.transition_status = TRANSITION_COMPLETE.to_string();
-        fi.transitioned_objname = dest_obj;
-        fi.transition_tier = opts.transition.tier.clone();
-        fi.transition_version_id = if rv.is_empty() { None } else { Some(Uuid::parse_str(&rv)?) };
+        drop(tier_config_mgr);
+
+        let transition_version_id = match parse_transition_version_id(&rv) {
+            Ok(version_id) => version_id,
+            Err(err) => {
+                cleanup_uncommitted_transition_upload(&opts.transition.tier, &dest_obj, &rv).await;
+                return Err(err.into());
+            }
+        };
+        let cleanup_remote_version = transition_cleanup_remote_version(&rv, transition_version_id);
+        let mut upload_cleanup = TransitionUploadCleanup::new(&opts.transition.tier, &dest_obj, cleanup_remote_version);
+
+        let mut commit_opts = opts.clone();
+        commit_opts.no_lock = true;
+        commit_opts.metadata_cache_safe = false;
+        let transition_lock_guard = if opts.no_lock {
+            None
+        } else {
+            match self.acquire_write_lock_diag("transition_object_commit", bucket, object).await {
+                Ok(guard) => Some(guard),
+                Err(err) => {
+                    upload_cleanup.cleanup().await;
+                    return Err(err);
+                }
+            }
+        };
+        self.invalidate_get_object_metadata_cache(bucket, object).await;
+        let current = self.get_object_fileinfo(bucket, object, &commit_opts, true, false).await;
+        let (mut current_fi, _, _) = match current {
+            Ok(current) => current,
+            Err(err) => {
+                drop(transition_lock_guard);
+                upload_cleanup.cleanup().await;
+                return Err(err);
+            }
+        };
+        let source_matches = current_fi.version_id == fi.version_id
+            && current_fi.data_dir == fi.data_dir
+            && current_fi.mod_time == fi.mod_time
+            && current_fi.size == fi.size
+            && rustfs_utils::path::trim_etag(&get_raw_etag(&current_fi.metadata)) == stored_etag;
+        if current_fi.transition_status == TRANSITION_COMPLETE || !source_matches {
+            let already_transitioned = current_fi.transition_status == TRANSITION_COMPLETE;
+            drop(transition_lock_guard);
+            upload_cleanup.cleanup().await;
+            if already_transitioned {
+                return Ok(());
+            }
+            return Err(to_object_err(Error::other(DiskError::FileNotFound), vec![bucket, object]));
+        }
+
+        current_fi.transition_status = TRANSITION_COMPLETE.to_string();
+        current_fi.transitioned_objname = dest_obj;
+        current_fi.transition_tier = opts.transition.tier.clone();
+        current_fi.transition_version_id = transition_version_id;
+        fi = current_fi;
        let event_name = EventName::LifecycleTransition.as_str();
-        let mut should_notify_transition = true;
-
-        let disks = self.disk_inventory().await;

+        if transition_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) {
+            drop(transition_lock_guard);
+            upload_cleanup.cleanup().await;
+            return Err(StorageError::NamespaceLockQuorumUnavailable {
+                mode: "transition_object_commit",
+                bucket: bucket.to_string(),
+                object: object.to_string(),
+                required: 1,
+                achieved: 0,
+            });
+        }
+        #[cfg(test)]
+        pause_transition_before_local_commit(bucket, object).await;
+        upload_cleanup.disarm();
        if let Err(err) = self.delete_object_version(bucket, object, &fi, false).await {
-            should_notify_transition = false;
            warn!(
                bucket = bucket,
                object = object,
                error = ?err,
-                "transition completed on remote tier but source cleanup failed; skipping external lifecycle transition notification"
+                "transition remote upload completed but local commit failed"
            );
-        } else {
-            self.record_capacity_scope_if_needed(opts.capacity_scope_token, &disks);
+            self.invalidate_get_object_metadata_cache(bucket, object).await;
+            drop(transition_lock_guard);
+            return Err(err);
        }

        // delete_object_version persisted transition_status=complete and freed the
@@ -2361,6 +2616,9 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
        // early-return above and streams the already-deleted local data to the
        // remote tier again (rustfs/rustfs#4827).
        self.invalidate_get_object_metadata_cache(bucket, object).await;
+        drop(transition_lock_guard);
+        let disks = self.disk_inventory().await;
+        self.record_capacity_scope_if_needed(opts.capacity_scope_token, &disks);

        for disk in disks.iter() {
            if let Some(disk) = disk {
@@ -2372,17 +2630,15 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
            break;
        }

-        if should_notify_transition {
-            let obj_info = ObjectInfo::from_file_info(&fi, bucket, object, opts.versioned || opts.version_suspended);
-            send_event(EventArgs {
-                event_name: event_name.to_string(),
-                bucket_name: bucket.to_string(),
-                object: obj_info,
-                user_agent: "Internal: [ILM-Transition]".to_string(),
-                host: runtime_sources::default_local_node_name(),
-                ..Default::default()
-            });
-        }
+        let obj_info = ObjectInfo::from_file_info(&fi, bucket, object, opts.versioned || opts.version_suspended);
+        send_event(EventArgs {
+            event_name: event_name.to_string(),
+            bucket_name: bucket.to_string(),
+            object: obj_info,
+            user_agent: "Internal: [ILM-Transition]".to_string(),
+            host: runtime_sources::default_local_node_name(),
+            ..Default::default()
+        });
        //let tags = opts.lifecycle_audit_event.tags();
        //auditLogLifecycle(ctx, objInfo, ILMTransition, tags, traceFn)
        Ok(())
@@ -2738,6 +2994,127 @@ pub(in crate::set_disk::ops) mod hermetic_set_disks_support {
    }
 }

+#[cfg(all(test, feature = "test-util"))]
+mod transition_commit_failure_tests {
+    use super::hermetic_set_disks_support::hermetic_set_disks;
+    use super::*;
+    use crate::bucket::lifecycle::lifecycle::{TRANSITION_PENDING, TransitionOptions};
+    use crate::disk::DiskAPI as _;
+    use crate::services::tier::test_util::register_mock_tier;
+    use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations as _};
+    use http::HeaderMap;
+    use tokio::io::AsyncReadExt;
+
+    #[tokio::test]
+    #[serial_test::serial]
+    async fn local_commit_failure_returns_error_and_preserves_remote_candidate() {
+        let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await;
+        let bucket = "transition-commit-failure-bucket";
+        let object = "object.bin";
+        let payload = b"transition commit failure must preserve the uploaded candidate".repeat(1024);
+        for disk in &disk_stores {
+            disk.make_volume(bucket).await.expect("bucket volume should be created");
+        }
+
+        let mut reader = PutObjReader::from_vec(payload.clone());
+        let original = set_disks
+            .put_object(bucket, object, &mut reader, &ObjectOptions::default())
+            .await
+            .expect("source object should be written");
+        let (fi, parts_metadata, online_disks) = set_disks
+            .get_object_fileinfo(bucket, object, &ObjectOptions::default(), true, false)
+            .await
+            .expect("source metadata should resolve");
+        let generation = set_disks
+            .get_object_metadata_cache_generation(bucket, object)
+            .expect("metadata cache generation should be active");
+        let cache_key = GetObjectMetadataCacheKey::new(bucket, object, generation);
+        set_disks
+            .get_object_metadata_cache
+            .insert(
+                cache_key.clone(),
+                Arc::new(GetObjectMetadataCacheEntry {
+                    created_at: Instant::now(),
+                    fi: fi.clone(),
+                    parts_metadata,
+                    online_disks,
+                    read_quorum: 2,
+                }),
+            )
+            .await;
+        assert!(set_disks.get_object_metadata_cache.get(&cache_key).await.is_some());
+
+        let tier_name = format!("COLDTIER{}", &Uuid::new_v4().simple().to_string()[..8]).to_uppercase();
+        let backend = register_mock_tier(&runtime_sources::global_tier_config_mgr(), &tier_name).await;
+        let opts = ObjectOptions {
+            no_lock: true,
+            transition: TransitionOptions {
+                status: TRANSITION_PENDING.to_string(),
+                tier: tier_name,
+                etag: original.etag.clone().unwrap_or_default(),
+                ..Default::default()
+            },
+            version_id: original.version_id.map(|version| version.to_string()),
+            mod_time: original.mod_time,
+            ..Default::default()
+        };
+
+        let barrier = TransitionCommitBarrier::install(bucket, object);
+        let invalidations = MetadataCacheInvalidationProbe::install(bucket, object);
+        let transition_set = Arc::clone(&set_disks);
+        let transition = tokio::spawn(async move { transition_set.transition_object(bucket, object, &opts).await });
+        barrier.wait_until_paused().await;
+        assert_eq!(invalidations.count(), 1, "precommit revalidation must fence the old metadata once");
+        let saved_disks = {
+            let mut disks = set_disks.disks.write().await;
+            let saved = std::mem::take(&mut *disks);
+            *disks = vec![None; saved.len()];
+            saved
+        };
+        barrier.release();
+        let result = transition.await.expect("transition task should not panic");
+        *set_disks.disks.write().await = saved_disks;
+
+        result.expect_err("local write quorum failure must be returned to the transition worker");
+        assert_eq!(
+            invalidations.count(),
+            2,
+            "local commit failure must fence any partially committed metadata again"
+        );
+        assert_eq!(backend.put_count().await, 1);
+        assert_eq!(
+            backend.remove_count().await,
+            0,
+            "ambiguous local commit failure must retain the remote candidate"
+        );
+        assert_eq!(backend.object_count().await, 1);
+        assert!(
+            set_disks.get_object_metadata_cache.get(&cache_key).await.is_none(),
+            "local commit failure must invalidate pre-transition metadata"
+        );
+
+        let mut restored = Vec::new();
+        set_disks
+            .get_object_reader(
+                bucket,
+                object,
+                None,
+                HeaderMap::new(),
+                &ObjectOptions {
+                    no_lock: true,
+                    ..Default::default()
+                },
+            )
+            .await
+            .expect("the local source must remain readable after a fail-before-commit result")
+            .stream
+            .read_to_end(&mut restored)
+            .await
+            .expect("the local source body should drain");
+        assert_eq!(restored, payload);
+    }
+}
+
 #[cfg(test)]
 mod put_object_tmp_cleanup_tests {
    //! Regression coverage for backlog#924 (HP-3): the speculative tmp-dir