mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-12 16:16:55 +00:00
fix(object-data-cache): make memory container-aware and bound cache config (#4655)
* fix(object-data-cache): bound cache config and make memory container-aware Harden the object data cache configuration path so a single bad input degrades to the disabled adapter instead of OOM-killing pods, panicking at boot, or silently mis-sizing the cache (backlog#1110/1113/1114/1115/ 1127/1140/1130). - ODC-05: resolve capacity and the fill gate from the effective (container-aware) memory. Prefer sysinfo cgroup_limits() and use min(host, cgroup) as the total plus cgroup-derived availability, falling back to host values when no cgroup limit constrains. Log the resolved capacity and its basis once at startup. - ODC-08: cap ttl/time_to_idle at 30 days in validate() so an unbounded Duration can no longer trip moka's ~1000-year builder assertion. - ODC-09: drop the max_entry_bytes floor from the derived-capacity clamp so MAX_ENTRY_BYTES can no longer inflate total capacity above the safety clamp; reject an entry larger than the resolved capacity instead. - ODC-10: require an explicit max_bytes to clear max_entry_bytes plus the weigher overhead so a fillable-but-unretainable cache is rejected. - ODC-22: reject max_entry_bytes at/above the u32 weigher boundary. - ODC-35: warn (not reject) when time_to_idle exceeds ttl and document the min(ttl, time_to_idle) expiry interaction. - ODC-25: give numeric env overrides two-valued semantics via a new rustfs_utils::get_env_parse_outcome; a malformed value now disables the whole cache with one aggregated warning instead of silently keeping defaults. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): require identity_keys_max of at least 2 The identity index admits a new key by evicting the oldest one, so a budget of 1 evicts the previous key on every fill and can never hold two live keys of one object at once — a versioned bucket alternating two versions then hits a permanent 0% hit rate. Reject the degenerate value at config validation time, where the adapter turns it into a disabled cache with a warning, since the bounded-eviction policy cannot rescue it at runtime. Handed off from the identity-index batch (backlog#1128), which changed the overflow policy but could not touch validate(). Refs: backlog#1115, backlog#1128 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): let a zero free-memory floor opt out of the gate Making the memory gate container-aware turned it live in CI: the runners are Kubernetes pods, so the gate now reads the pod's cgroup free memory, finds it below the 20% floor, and refuses the fill. Tests that assert a fill succeeds then failed on CI while passing on a developer host, which has no cgroup and falls back to host memory. The gate is behaving correctly — the tests were the ones depending on a live memory reading. Treat min_free_memory_percent = 0 as a deliberate opt-out rather than an invalid value: allows_fill returns early before it touches any snapshot, so admission becomes independent of where the suite runs. Operators gain the same escape hatch. Every test that requires a fill to succeed now sets the floor to 0. The tests that exercise the gate keep it enabled via memory_gated_config, so the ODC-05 coverage they provide is preserved rather than short-circuited. Refs: backlog#1110 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): opt the usecase fill tests out of the memory gate The previous commit exempted the fill-dependent tests it could find, but missed the six adapters built inside object_usecase.rs. Fixing the first batch moved nextest's fail-fast point forward and CI surfaced them: build_get_object_body_with_cache_materializes_once_and_hits_later and ..._uses_cached_body_without_reader_preread both assert a fill lands, and both ran with the default 20% free-memory floor. Set the floor to 0 on all six, matching the sibling test modules. Verified by pinning the gate's snapshot to 0% available — harsher than any CI pod — and confirming these tests still pass, which shows the exemption path never reads memory at all. An exhaustive sweep over every fill-enabled ObjectDataCacheConfig in the tree now shows no remaining site: the only unexempted ones are fill_enabled()'s matches! arm and two adapter tests that assert the adapter is disabled and therefore never fill. Refs: backlog#1110 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
houseme
@@ -52,8 +52,21 @@ impl ObjectDataCacheAdapter {
|
||||
|
||||
/// Creates an adapter from runtime environment variables, falling back to
|
||||
/// no-op on invalid input so startup behavior stays conservative.
|
||||
///
|
||||
/// A malformed value on any single key (bad mode string, failed validation,
|
||||
/// or an unparseable numeric override) disables the whole cache rather than
|
||||
/// silently starting with defaults the operator never chose.
|
||||
pub(crate) fn from_env_or_disabled() -> Arc<Self> {
|
||||
Self::from_config_or_disabled(object_data_cache_config_from_env(), "env")
|
||||
match object_data_cache_config_from_env() {
|
||||
Ok(config) => Self::from_config_or_disabled(config, "env"),
|
||||
Err(invalid_keys) => {
|
||||
warn!(
|
||||
invalid_keys = %invalid_keys.join(","),
|
||||
"object data cache disabled because one or more environment variables are malformed"
|
||||
);
|
||||
Self::disabled_arc()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Creates a disabled no-op adapter.
|
||||
@@ -131,20 +144,51 @@ impl Default for ObjectDataCacheAdapter {
|
||||
}
|
||||
}
|
||||
|
||||
fn object_data_cache_config_from_env() -> ObjectDataCacheConfig {
|
||||
object_data_cache_config_from_values(ObjectDataCacheEnvValues {
|
||||
/// Reads a numeric env override with three-valued semantics: absent keeps the
|
||||
/// default (`None`), a valid value is applied, and a malformed value records the
|
||||
/// key in `invalid_keys` so the caller can invalidate the whole config.
|
||||
fn take_numeric_env<T>(key: &'static str, invalid_keys: &mut Vec<&'static str>) -> Option<T>
|
||||
where
|
||||
T: std::str::FromStr,
|
||||
{
|
||||
match rustfs_utils::get_env_parse_outcome::<T>(key) {
|
||||
rustfs_utils::EnvParseOutcome::Parsed(value) => Some(value),
|
||||
rustfs_utils::EnvParseOutcome::Absent => None,
|
||||
rustfs_utils::EnvParseOutcome::Invalid => {
|
||||
invalid_keys.push(key);
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn object_data_cache_config_from_env() -> Result<ObjectDataCacheConfig, Vec<&'static str>> {
|
||||
let mut invalid_keys: Vec<&'static str> = Vec::new();
|
||||
|
||||
let values = ObjectDataCacheEnvValues {
|
||||
enabled: rustfs_utils::get_env_opt_bool(rustfs_config::ENV_OBJECT_DATA_CACHE_ENABLE),
|
||||
mode: rustfs_utils::get_env_opt_str(rustfs_config::ENV_OBJECT_DATA_CACHE_MODE),
|
||||
max_bytes: rustfs_utils::get_env_opt_u64(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_BYTES),
|
||||
max_memory_percent: rustfs_utils::get_env_opt_u8(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_MEMORY_PERCENT),
|
||||
max_entry_bytes: rustfs_utils::get_env_opt_u64(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_ENTRY_BYTES),
|
||||
ttl_secs: rustfs_utils::get_env_opt_u64(rustfs_config::ENV_OBJECT_DATA_CACHE_TTL_SECS),
|
||||
time_to_idle_secs: rustfs_utils::get_env_opt_u64(rustfs_config::ENV_OBJECT_DATA_CACHE_TIME_TO_IDLE_SECS),
|
||||
min_free_memory_percent: rustfs_utils::get_env_opt_u8(rustfs_config::ENV_OBJECT_DATA_CACHE_MIN_FREE_MEMORY_PERCENT),
|
||||
fill_concurrency_per_cpu: rustfs_utils::get_env_opt_u16(rustfs_config::ENV_OBJECT_DATA_CACHE_FILL_CONCURRENCY_PER_CPU),
|
||||
fill_concurrency_max: rustfs_utils::get_env_opt_u16(rustfs_config::ENV_OBJECT_DATA_CACHE_FILL_CONCURRENCY_MAX),
|
||||
identity_keys_max: rustfs_utils::get_env_opt_u16(rustfs_config::ENV_OBJECT_DATA_CACHE_IDENTITY_KEYS_MAX),
|
||||
})
|
||||
max_bytes: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_BYTES, &mut invalid_keys),
|
||||
max_memory_percent: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_MEMORY_PERCENT, &mut invalid_keys),
|
||||
max_entry_bytes: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_ENTRY_BYTES, &mut invalid_keys),
|
||||
ttl_secs: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_TTL_SECS, &mut invalid_keys),
|
||||
time_to_idle_secs: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_TIME_TO_IDLE_SECS, &mut invalid_keys),
|
||||
min_free_memory_percent: take_numeric_env(
|
||||
rustfs_config::ENV_OBJECT_DATA_CACHE_MIN_FREE_MEMORY_PERCENT,
|
||||
&mut invalid_keys,
|
||||
),
|
||||
fill_concurrency_per_cpu: take_numeric_env(
|
||||
rustfs_config::ENV_OBJECT_DATA_CACHE_FILL_CONCURRENCY_PER_CPU,
|
||||
&mut invalid_keys,
|
||||
),
|
||||
fill_concurrency_max: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_FILL_CONCURRENCY_MAX, &mut invalid_keys),
|
||||
identity_keys_max: take_numeric_env(rustfs_config::ENV_OBJECT_DATA_CACHE_IDENTITY_KEYS_MAX, &mut invalid_keys),
|
||||
};
|
||||
|
||||
if !invalid_keys.is_empty() {
|
||||
return Err(invalid_keys);
|
||||
}
|
||||
|
||||
Ok(object_data_cache_config_from_values(values))
|
||||
}
|
||||
|
||||
fn object_data_cache_config_from_values(values: ObjectDataCacheEnvValues) -> ObjectDataCacheConfig {
|
||||
@@ -217,11 +261,41 @@ fn parse_object_data_cache_mode(value: &str) -> Option<ObjectDataCacheMode> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{
|
||||
ObjectDataCacheAdapter, ObjectDataCacheEnvValues, object_data_cache_config_from_values, parse_object_data_cache_mode,
|
||||
ObjectDataCacheAdapter, ObjectDataCacheEnvValues, object_data_cache_config_from_env,
|
||||
object_data_cache_config_from_values, parse_object_data_cache_mode,
|
||||
};
|
||||
use rustfs_object_data_cache::{ObjectDataCacheConfig, ObjectDataCacheMode};
|
||||
use std::time::Duration;
|
||||
|
||||
/// All object-data-cache env keys, unset by default. Tests override the
|
||||
/// entries they care about so the surrounding process environment cannot
|
||||
/// influence the outcome.
|
||||
fn all_env_unset() -> Vec<(&'static str, Option<&'static str>)> {
|
||||
vec![
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_ENABLE, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MODE, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_BYTES, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_MEMORY_PERCENT, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_ENTRY_BYTES, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_TTL_SECS, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_TIME_TO_IDLE_SECS, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MIN_FREE_MEMORY_PERCENT, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_FILL_CONCURRENCY_PER_CPU, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_FILL_CONCURRENCY_MAX, None),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_IDENTITY_KEYS_MAX, None),
|
||||
]
|
||||
}
|
||||
|
||||
fn with_env_overrides(overrides: &[(&'static str, &'static str)]) -> Vec<(&'static str, Option<&'static str>)> {
|
||||
let mut vars = all_env_unset();
|
||||
for (key, value) in overrides {
|
||||
if let Some(entry) = vars.iter_mut().find(|(name, _)| name == key) {
|
||||
entry.1 = Some(value);
|
||||
}
|
||||
}
|
||||
vars
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn disabled_adapter_exposes_disabled_engine() {
|
||||
let adapter = ObjectDataCacheAdapter::disabled();
|
||||
@@ -342,4 +416,70 @@ mod tests {
|
||||
|
||||
assert!(adapter.is_disabled());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn overlong_ttl_builds_disabled_adapter_without_panic() {
|
||||
// moka's builder asserts on durations beyond ~1000 years; validate()
|
||||
// must reject an out-of-range ttl so the adapter degrades to disabled
|
||||
// rather than panicking AppContext::new at boot.
|
||||
let adapter = ObjectDataCacheAdapter::from_config_or_disabled(
|
||||
ObjectDataCacheConfig {
|
||||
mode: ObjectDataCacheMode::FillMaterializeEnabled,
|
||||
max_bytes: 8 * 1024 * 1024,
|
||||
ttl: Duration::from_secs(u64::MAX),
|
||||
..ObjectDataCacheConfig::default()
|
||||
},
|
||||
"test",
|
||||
);
|
||||
|
||||
assert!(adapter.is_disabled());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_data_cache_env_absent_uses_defaults() {
|
||||
temp_env::with_vars(all_env_unset(), || {
|
||||
let config = object_data_cache_config_from_env().expect("absent env should build the default config");
|
||||
|
||||
assert_eq!(config, ObjectDataCacheConfig::default());
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_data_cache_env_valid_numeric_value_is_applied() {
|
||||
let vars = with_env_overrides(&[
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_ENABLE, "true"),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_ENTRY_BYTES, "2097152"),
|
||||
]);
|
||||
temp_env::with_vars(vars, || {
|
||||
let config = object_data_cache_config_from_env().expect("valid numeric env should be applied");
|
||||
|
||||
assert_eq!(config.mode, ObjectDataCacheMode::HitOnly);
|
||||
assert_eq!(config.max_entry_bytes, 2_097_152);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_data_cache_env_malformed_numeric_invalidates_config() {
|
||||
let key = rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_ENTRY_BYTES;
|
||||
let vars = with_env_overrides(&[(rustfs_config::ENV_OBJECT_DATA_CACHE_ENABLE, "true"), (key, "not-a-number")]);
|
||||
temp_env::with_vars(vars, || {
|
||||
let invalid_keys =
|
||||
object_data_cache_config_from_env().expect_err("a malformed numeric key must invalidate the config");
|
||||
|
||||
assert!(invalid_keys.contains(&key));
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_data_cache_env_malformed_numeric_builds_disabled_adapter() {
|
||||
let vars = with_env_overrides(&[
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_ENABLE, "true"),
|
||||
(rustfs_config::ENV_OBJECT_DATA_CACHE_MAX_ENTRY_BYTES, "not-a-number"),
|
||||
]);
|
||||
temp_env::with_vars(vars, || {
|
||||
let adapter = ObjectDataCacheAdapter::from_env_or_disabled();
|
||||
|
||||
assert!(adapter.is_disabled());
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -92,6 +92,8 @@ mod tests {
|
||||
let config = ObjectDataCacheConfig {
|
||||
mode: ObjectDataCacheMode::FillBufferedOnly,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..ObjectDataCacheConfig::default()
|
||||
};
|
||||
ObjectDataCacheAdapter::new(config).expect("fill-enabled config should build adapter")
|
||||
|
||||
@@ -86,6 +86,8 @@ mod tests {
|
||||
ObjectDataCacheAdapter::new(ObjectDataCacheConfig {
|
||||
mode: ObjectDataCacheMode::FillBufferedOnly,
|
||||
max_bytes: 4 * 1024 * 1024,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("adapter"),
|
||||
|
||||
@@ -128,6 +128,8 @@ mod tests {
|
||||
let config = ObjectDataCacheConfig {
|
||||
mode: ObjectDataCacheMode::FillMaterializeEnabled,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..ObjectDataCacheConfig::default()
|
||||
};
|
||||
ObjectDataCacheAdapter::new(config).expect("enabled config should build adapter")
|
||||
|
||||
@@ -7463,6 +7463,8 @@ mod tests {
|
||||
crate::app::object_data_cache::ObjectDataCacheAdapter::new(rustfs_object_data_cache::ObjectDataCacheConfig {
|
||||
mode: rustfs_object_data_cache::ObjectDataCacheMode::FillBufferedOnly,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..rustfs_object_data_cache::ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("fill-enabled cache adapter should initialize");
|
||||
@@ -7519,6 +7521,8 @@ mod tests {
|
||||
crate::app::object_data_cache::ObjectDataCacheAdapter::new(rustfs_object_data_cache::ObjectDataCacheConfig {
|
||||
mode: rustfs_object_data_cache::ObjectDataCacheMode::FillBufferedOnly,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..rustfs_object_data_cache::ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("fill-enabled cache adapter should initialize");
|
||||
@@ -7583,6 +7587,8 @@ mod tests {
|
||||
crate::app::object_data_cache::ObjectDataCacheAdapter::new(rustfs_object_data_cache::ObjectDataCacheConfig {
|
||||
mode: rustfs_object_data_cache::ObjectDataCacheMode::FillBufferedOnly,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..rustfs_object_data_cache::ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("fill-enabled cache adapter should initialize");
|
||||
@@ -7652,6 +7658,8 @@ mod tests {
|
||||
crate::app::object_data_cache::ObjectDataCacheAdapter::new(rustfs_object_data_cache::ObjectDataCacheConfig {
|
||||
mode: rustfs_object_data_cache::ObjectDataCacheMode::FillBufferedOnly,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..rustfs_object_data_cache::ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("fill-enabled cache adapter should initialize");
|
||||
@@ -7715,6 +7723,8 @@ mod tests {
|
||||
crate::app::object_data_cache::ObjectDataCacheAdapter::new(rustfs_object_data_cache::ObjectDataCacheConfig {
|
||||
mode: rustfs_object_data_cache::ObjectDataCacheMode::FillMaterializeEnabled,
|
||||
max_bytes: 8_388_608,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..rustfs_object_data_cache::ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("materialize-fill cache adapter should initialize");
|
||||
@@ -7786,6 +7796,8 @@ mod tests {
|
||||
mode: rustfs_object_data_cache::ObjectDataCacheMode::FillMaterializeEnabled,
|
||||
max_bytes: 8_388_608,
|
||||
max_entry_bytes: 4,
|
||||
// Fill must not depend on the live memory reading (host vs container).
|
||||
min_free_memory_percent: 0,
|
||||
..rustfs_object_data_cache::ObjectDataCacheConfig::default()
|
||||
})
|
||||
.expect("materialize-fill cache adapter should initialize");
|
||||
|
||||
Reference in New Issue
Block a user