mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-04 20:37:43 +00:00
d29e637890
fix(object-data-cache): close identity-index races in fill/invalidate (backlog#1117/#1118/#1125/#1128/#1136) The object body cache's fill-vs-invalidation contract (index.insert before cache.insert, then re-check index membership and undo the fill on mismatch) had several races where the identity index and the cache could drift, either poisoning the race metric, breaking invalidation silently, or letting a principal evict a hot body on demand. ODC-12 (backlog#1117) — generation-aware index removal. The eviction listener removed (identity, key) by key equality with no way to tell an evicted old entry from a freshly refilled one under the same key. Verified against moka 0.12.15: an insert over a TTL/TTI-expired but not yet purged entry runs do_insert_with_hash inline during insert().await, and do_post_update_steps awaits notify_upsert INLINE with cause Expired (was_evicted() == true), so the listener deleted the index key the refill just registered and the post-insert recheck then self-destructed the fresh entry (SkippedInvalidationRace). A deferred Size notification for an old generation had the same effect on a cleanly refilled key. Chose Option A (generation token) over Option B (re-insert after cache.contains_key): B removes the key first and only reconciles afterward, which reopens a narrow window where a concurrent invalidate_object misses the key while it is transiently absent from the index. A never removes a key whose generation does not match, so the decision is atomic under the shard lock. The token is the cache entry's Arc pointer captured at fill time (readable from the value moka hands the listener), so no change to the entry type is needed. ODC-13 (backlog#1118) — cancellation-safe insert/recheck/undo. The recheck+undo sat after two awaits inside the S3 GET future; a client disconnect could cancel the fill at the recheck await after cache.insert made the entry visible, stranding a stale body with no index entry. The register/insert/recheck/undo sequence now runs in a spawned task whose JoinHandle the leader awaits: cancelling the await detaches the task, which still finishes the recheck and undo. The dropped leader unblocks waiters as before (SkippedSingleflightClosed), so no waiter is stranded. ODC-20 (backlog#1125) — do not prune in-flight sibling keys. prune_missing could remove another in-flight fill's index key (a different key of the same identity that had registered but not yet published its cache entry), making that fill's recheck fail and delete its own valid entry. Added a read-only ObjectDataCacheSingleflight::has_inflight and consult it in the prune predicate alongside cache.contains_key. ODC-23 (backlog#1128) — bounded eviction instead of clear-and-reject. Exceeding identity_keys_max drained the whole key set and rejected the incoming key, so a principal able to GET identity_keys_max+1 distinct versions could evict an object's hot body on demand, and identity_keys_max=1 on a versioned bucket meant a permanent 0% hit rate. The key set now evicts only the oldest key(s) (the Vec preserves insertion order) and inserts the new key. The insert result carries evicted_keys so the backend removes exactly those from the cache and still caches the newest body. The Overflow variant and the now-unused SkippedIdentityOverflow construction are gone. NOTE: the audit also asks for identity_keys_max >= 2 validation; that lives in config.rs (out of scope here) and is deferred to the config batch. ODC-31 (backlog#1136) — deterministic race coverage. Added a #[cfg(test)] fill barrier between the index insert and the cache insert so the fill-vs-invalidation recheck can be driven deterministically, plus a companion test asserting the identity-budget eviction drops the evicted keys' cache entries. New tests: - index: key_set_bounded_eviction_evicts_oldest_and_inserts_new, key_set_removes_only_matching_generation_token, key_set_duplicate_refreshes_generation_token - starshard_index: identity_index_bounded_eviction_evicts_oldest, identity_index_stale_eviction_preserves_refreshed_key, identity_index_matching_eviction_removes_key - moka_backend: moka_backend_refill_after_expiry_without_maintenance_inserts, moka_backend_concurrent_fills_same_identity_both_insert, moka_backend_identity_budget_evicts_oldest_and_caches_newest, moka_backend_fill_loses_race_to_invalidation, moka_backend_cancelled_fill_still_undoes_lost_race cargo fmt --all, cargo check/clippy/test -p rustfs-object-data-cache all pass (44 tests). Verified the refill-after-expiry and concurrent-fill tests fail without their respective fixes. Co-authored-by: heihutu <heihutu@gmail.com>
189 lines
6.6 KiB
Rust
189 lines
6.6 KiB
Rust
// 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 crate::key::ObjectDataCacheKey;
|
|
use crate::starshard_index::StarshardIdentityIndex;
|
|
|
|
/// Per-entry generation token used to tell a freshly refilled body apart from a
|
|
/// superseded one under the same key. The token is the cache entry's `Arc`
|
|
/// pointer captured at fill time: the eviction listener receives the evicted
|
|
/// value and can compare its pointer against the token currently tracked, so a
|
|
/// deferred or inline eviction of an old generation cannot remove the index key
|
|
/// registered for the current generation.
|
|
pub(crate) type ObjectDataCacheKeyToken = usize;
|
|
|
|
/// Result of inserting a cache key into the identity index.
|
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
|
pub enum ObjectDataCacheIndexInsertResult {
|
|
/// The key was inserted into the identity set.
|
|
Inserted {
|
|
/// Oldest keys evicted to keep the identity within its key budget.
|
|
///
|
|
/// Empty unless the identity was already at `identity_keys_max`; the
|
|
/// caller must drop these keys from the cache.
|
|
evicted_keys: Vec<ObjectDataCacheKey>,
|
|
},
|
|
/// The key was already tracked for the identity; its generation token was refreshed.
|
|
Duplicate,
|
|
}
|
|
|
|
#[derive(Debug, Clone)]
|
|
struct TrackedKey {
|
|
key: ObjectDataCacheKey,
|
|
token: ObjectDataCacheKeyToken,
|
|
}
|
|
|
|
#[derive(Debug, Clone, Default)]
|
|
pub(crate) struct ObjectDataCacheKeySet {
|
|
keys: Vec<TrackedKey>,
|
|
}
|
|
|
|
impl ObjectDataCacheKeySet {
|
|
pub(crate) fn insert(
|
|
&mut self,
|
|
key: ObjectDataCacheKey,
|
|
token: ObjectDataCacheKeyToken,
|
|
max_keys: usize,
|
|
) -> ObjectDataCacheIndexInsertResult {
|
|
if let Some(existing) = self.keys.iter_mut().find(|existing| existing.key == key) {
|
|
// Refresh the generation token so a later eviction of the superseded
|
|
// body cannot remove the key registered for this new body.
|
|
existing.token = token;
|
|
return ObjectDataCacheIndexInsertResult::Duplicate;
|
|
}
|
|
|
|
// Bounded eviction: the Vec preserves insertion order, so evicting from
|
|
// the front drops the oldest keys and keeps hot (recently filled) ones,
|
|
// rather than clearing the whole identity and rejecting the new key.
|
|
let mut evicted_keys = Vec::new();
|
|
while self.keys.len() >= max_keys {
|
|
evicted_keys.push(self.keys.remove(0).key);
|
|
}
|
|
|
|
self.keys.push(TrackedKey { key, token });
|
|
ObjectDataCacheIndexInsertResult::Inserted { evicted_keys }
|
|
}
|
|
|
|
/// Removes the key only when its tracked generation token matches, so an
|
|
/// eviction notification for an old generation leaves a refreshed key intact.
|
|
pub(crate) fn remove_evicted_key(&mut self, key: &ObjectDataCacheKey, token: ObjectDataCacheKeyToken) -> bool {
|
|
let original_len = self.keys.len();
|
|
self.keys
|
|
.retain(|existing| !(existing.key == *key && existing.token == token));
|
|
original_len != self.keys.len()
|
|
}
|
|
|
|
pub(crate) fn retain<F>(&mut self, mut keep: F)
|
|
where
|
|
F: FnMut(&ObjectDataCacheKey) -> bool,
|
|
{
|
|
self.keys.retain(|tracked| keep(&tracked.key));
|
|
}
|
|
|
|
pub(crate) fn is_empty(&self) -> bool {
|
|
self.keys.is_empty()
|
|
}
|
|
|
|
pub(crate) fn contains(&self, key: &ObjectDataCacheKey) -> bool {
|
|
self.keys.iter().any(|existing| &existing.key == key)
|
|
}
|
|
|
|
#[cfg(test)]
|
|
pub(crate) fn len(&self) -> usize {
|
|
self.keys.len()
|
|
}
|
|
|
|
pub(crate) fn cloned(&self) -> Vec<ObjectDataCacheKey> {
|
|
self.keys.iter().map(|tracked| tracked.key.clone()).collect()
|
|
}
|
|
}
|
|
|
|
/// Public identity-index façade used by the cache backend.
|
|
pub type ObjectDataCacheIdentityIndex = StarshardIdentityIndex;
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::{ObjectDataCacheIndexInsertResult, ObjectDataCacheKeySet};
|
|
use crate::key::{ObjectDataCacheBodyVariant, ObjectDataCacheKey};
|
|
|
|
fn make_key(id: &str) -> ObjectDataCacheKey {
|
|
ObjectDataCacheKey::new("bucket", "object", Some(id), "etag", 1, ObjectDataCacheBodyVariant::FullObjectPlainV1)
|
|
}
|
|
|
|
#[test]
|
|
fn key_set_deduplicates_existing_key() {
|
|
let mut set = ObjectDataCacheKeySet::default();
|
|
let key = make_key("v1");
|
|
|
|
let first = set.insert(key.clone(), 1, 4);
|
|
let second = set.insert(key, 2, 4);
|
|
|
|
assert_eq!(
|
|
first,
|
|
ObjectDataCacheIndexInsertResult::Inserted {
|
|
evicted_keys: Vec::new()
|
|
}
|
|
);
|
|
assert_eq!(second, ObjectDataCacheIndexInsertResult::Duplicate);
|
|
assert_eq!(set.len(), 1);
|
|
}
|
|
|
|
#[test]
|
|
fn key_set_bounded_eviction_evicts_oldest_and_inserts_new() {
|
|
let mut set = ObjectDataCacheKeySet::default();
|
|
let key_a = make_key("v1");
|
|
let key_b = make_key("v2");
|
|
|
|
let _ = set.insert(key_a.clone(), 1, 1);
|
|
let result = set.insert(key_b.clone(), 2, 1);
|
|
|
|
assert!(matches!(
|
|
result,
|
|
ObjectDataCacheIndexInsertResult::Inserted { evicted_keys } if evicted_keys == vec![key_a]
|
|
));
|
|
// The new key replaces the evicted one instead of the identity being cleared.
|
|
assert!(set.contains(&key_b));
|
|
assert_eq!(set.len(), 1);
|
|
}
|
|
|
|
#[test]
|
|
fn key_set_removes_only_matching_generation_token() {
|
|
let mut set = ObjectDataCacheKeySet::default();
|
|
let key = make_key("v1");
|
|
|
|
let _ = set.insert(key.clone(), 1, 4);
|
|
|
|
// A stale eviction notification carrying the old token must not remove the key.
|
|
assert!(!set.remove_evicted_key(&key, 2));
|
|
assert!(set.contains(&key));
|
|
|
|
// The matching token removes the key.
|
|
assert!(set.remove_evicted_key(&key, 1));
|
|
assert!(!set.contains(&key));
|
|
}
|
|
|
|
#[test]
|
|
fn key_set_duplicate_refreshes_generation_token() {
|
|
let mut set = ObjectDataCacheKeySet::default();
|
|
let key = make_key("v1");
|
|
|
|
let _ = set.insert(key.clone(), 1, 4);
|
|
let _ = set.insert(key.clone(), 2, 4);
|
|
|
|
// After the refresh the old token no longer matches.
|
|
assert!(!set.remove_evicted_key(&key, 1));
|
|
assert!(set.remove_evicted_key(&key, 2));
|
|
}
|
|
}
|