Commit Graph

4 Commits

Author SHA1 Message Date
houseme 15f4e75870 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-07-18 14:37:10 +00:00
houseme b604074217 perf(object-data-cache): take fill off the GET path and fix the metrics (#4678)
* fix(object-data-cache): make cache metrics one-increment-per-GET

Rework the object data cache observability so each counter carries one
clear meaning, and drop dead per-entry state.

ODC-17 (backlog#1122): split requests_total, which was incremented by
both plan_get and lookup_body, into plan_total{decision,reason,size_class}
(emitted only by plan_get) and lookup_total{result,size_class} (emitted
only by lookup_body). Each is now incremented exactly once per GET per
layer; help text states this.

ODC-18 (backlog#1123): add a JoinedInflightFill result (label
joined_inflight) so a singleflight waiter is no longer counted as an
insert. record_fill_result now always counts the outcome but records
fill bytes only when non-zero and the duration histogram only when
present, so joined_inflight / skipped_by_mode / skipped_size_mismatch
no longer inflate fill_bytes_total or add non-fill duration samples.
The waiter->JoinedInflightFill mapping lives in MokaBackend (owned by a
concurrent branch); cache.rs handles the variant already.

ODC-29 (backlog#1134): stop refreshing the cache-state gauge on every
lookup, and debounce it on fill/invalidate to at most once per second
via an AtomicU64 millis timestamp, since moka's entry_count is a
settling approximation.

ODC-36 (backlog#1141): give invalidations_total an outcome label
(removed|noop) and skip the gauge refresh on the no-op path. Extend
ObjectDataCacheInvalidationResult with Removed{keys}/NoOp (Success kept
as a transitional variant until MokaBackend reports the removal count);
NoopBackend now reports NoOp.

ODC-30 (backlog#1141): drop the dead content_length/etag/inserted_at
fields (and getters) from ObjectDataCacheEntry, which are redundant with
the moka key identity; the constructor keeps its arity so the
out-of-scope MokaBackend caller still compiles. Gate is_null_version
behind cfg(test).

Tests use a thread-local metrics recorder to avoid the global-singleton
flakiness. Fill-enabled test configs set min_free_memory_percent=0 so
the cgroup gate does not refuse fills in CI pods.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(object-data-cache): move fill off GET path, bound & harden the fill

Implements five object-data-cache audit findings.

ODC-15 (backlog#1120): the GET response no longer blocks on cache fill.
The buffered/materialized body is already in hand, so the fill now runs
in a detached task and the response is built immediately. Singleflight is
made non-blocking: `try_acquire` returns Leader or Busy, and a non-leader
skips its own fill (SkippedSingleflightBusy) instead of waiting on another
request's leader. The inner fill spawn is KEPT: once the index key is
registered, the recheck/undo must complete even if the enclosing fill
future is aborted, so removing it would weaken the cancellation-safety
guarantee (ODC-13 test) and the invalidation-race undo.

ODC-11 (backlog#1116): enforce the fill-concurrency knobs. MokaBackend
now holds a Semaphore sized min(per_cpu * parallelism, max), acquired
after winning leadership and before the memory gate. On saturation it
rejects (try_acquire_owned) with SkippedFillConcurrency rather than
queueing, so the fill path never reintroduces GET-latency coupling.

ODC-14 (backlog#1119): the memory gate no longer does a blocking sysinfo
refresh under a mutex on the async fill path. A dedicated periodic
refresher (tokio interval + spawn_blocking) updates an atomic snapshot
every 5s; allows_fill is now lock-free. The min_free_memory_percent == 0
short-circuit still runs before any snapshot read. No runtime at
construction (sync unit tests) simply keeps the seed snapshot.

ODC-32 (backlog#1137): singleflight no longer emits metrics while holding
the map mutex. try_acquire/remove_entry capture the map length under the
guard, drop it, then emit the gauge/counter.

ODC-07 (backlog#1112): the materialize read is bounded via
take(capacity + 1) so an over-long stream cannot grow the buffer past the
in-memory GET threshold, and any length mismatch is now a hard error
matching the direct-memory GET path, instead of warn-and-serve.

cache.rs is owned by a concurrent branch; this commit only appends the
SkippedFillConcurrency and SkippedSingleflightBusy variants + label arms.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(object-data-cache): report JoinedInflightFill and invalidation outcome

Reconciles the moka_backend half of two metrics-semantics findings after
merging fix/odc-metrics-semantics (which landed the cache.rs half).

ODC-18 (backlog#1123): the singleflight non-leader path now returns
JoinedInflightFill instead of counting as an insert, so N concurrent GETs
of one cold key record one insert, not N. This composes with the ODC-15
redesign: the non-leader does NOT wait for the leader (it already owns the
body and never re-serves from the fill result), so no blocking wait is
reintroduced. Drops the redundant local SkippedSingleflightBusy variant in
favor of the canonical JoinedInflightFill (mapped to no bytes / no duration
by the facade). SkippedFillConcurrency (ODC-11) is retained.

ODC-36 (backlog#1141): MokaBackend::invalidate_object now returns
Removed { keys } / NoOp instead of the transitional Success, so the facade
labels invalidations_total{outcome=removed|noop} and skips the cache-state
gauge refresh on the no-op path.

Tests: duplicate-fill test asserts JoinedInflightFill (bites when reverted
to Inserted); new no-op invalidation test; matching-identity test asserts
Removed { keys: 1 }.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(object-data-cache): drop unused mut on materialize stream binding

The ODC-07 change moves `final_stream` into `AsyncReadExt::take`, so the
`build_get_object_body_with_cache` parameter is no longer mutated in place.

Co-Authored-By: heihutu <heihutu@gmail.com>

* refactor(object-data-cache): close the cross-branch coordination seams

Merging the metrics and hot-path branches left four transitional shims that
only existed because each side could not edit the other's files. With both
sides present they are dead weight, and two of them actively misreport.

- ObjectDataCacheEntry::new took content_length and etag only to keep the
  caller in moka_backend.rs compiling, then discarded both. Drop them; the
  entry is a Bytes wrapper and the caller now passes only the body.

- SkippedSingleflightClosed lost its only producer when the waiter model was
  replaced by Leader/Busy election. Remove the variant.

- The fill-accounting match ended in a wildcard that charged fill_bytes and a
  duration to every non-Inserted outcome, so a fill rejected by the memory
  gate or the concurrency semaphore — which never touched the backend and
  wrote nothing — still inflated fill_bytes_total. Enumerate every variant
  explicitly: only outcomes that wrote the body report bytes and duration.
  Exhaustiveness also forces a future variant to state its accounting rather
  than inherit a wrong default.

- InvalidationResult::Success mapped a no-op to outcome=removed, the exact lie
  backlog#1141 set out to fix, and its doc comment claimed MokaBackend still
  returned it after that backend had been widened. It has no producer left.
  Remove it, and tighten the app-layer test from "any successful variant" to
  the real contract: the pre-mutation call reports Removed{keys:1}, the
  post-delete call NoOp.

Refs: backlog#1123, backlog#1141, backlog#1135

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-10 12:25:23 +00:00
houseme d29e637890 fix(object-data-cache): close identity-index races in fill/invalidate (#4657)
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>
2026-07-10 08:04:57 +00:00
houseme eebd16d8a4 feat(cache): add object data cache engine and app flow (#4187)
* feat(cache): add object data cache engine

* feat(cache): wire app-layer object cache flow

* refactor(cache): streamline app-layer cache flow

* refactor(cache): tighten cache flow internals

* refactor: address final clippy cleanup

* chore(deps): update quick-xml to 0.41.0

* feat(cache): wire object data cache env config

* fix(cache): gate materialize fill by cache plan

* chore(cache): add object data cache benchmark gate

* fix(cache): guard object cache fill size mismatches

* refactor(cache): streamline object cache body planning

* fix(cache): align object cache rollout config

* test(cache): cover buffered object cache benchmark

* test(cache): isolate object cache benchmark metrics

* test(cache): mark materialize rollout experimental

* test(cache): tighten object cache benchmark gate

* fix(cache): address review findings for object data cache

- singleflight: clean up leader entry on cancellation (Drop impl) so a dropped GET future can no longer wedge all subsequent fills for the same key; switch the fill map to a std Mutex and add a regression test

- adapter: honor RUSTFS_OBJECT_DATA_CACHE_ENABLE=true by defaulting to hit_only when no explicit mode is set (explicit mode still wins)

- planner: treat nil version UUIDs as "no value" per repo convention so unversioned objects key under the canonical "null" instead of fragmenting the key space

- multipart: invalidate the object cache on the quota-exceeded rollback delete after complete-multipart, closing a stale-cache window

- layering: move the disabled-cache fallback into app::context and drop the new infra->app layer-dependency baseline entry

* fix(cache): close invalidation races and drop full-cache scan on writes

- index: make identity-index insert/remove/prune atomic via starshard compute_if_present/compute_if_absent so concurrent fills can no longer drop each other's keys (lost keys made entries unreachable to invalidation until TTL); add a concurrency regression test

- fill: register the key in the identity index before the entry becomes visible in the cache and re-check the index afterwards, undoing the fill when an invalidation raced in between (new skipped_invalidation_race fill result)

- invalidate: with the index now authoritative, remove the full-cache iter() fallback that made every PUT/DELETE of a never-cached object O(total cache entries) (two scans per PUT, 2N per batch delete)

- materialize-fill: fail the GET instead of falling back to the partially consumed stream after a mid-read error (the fallback would send a body missing its prefix under a full-length Content-Length), and log the same size-mismatch warning as the sibling buffering paths

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(storage): fix media-dependent buffer clamp expectation

test_concurrency_manager_multi_factor_strategy_buffer_clamp asserted media_cap.min(MI_B), but the implementation's final safety clamp is [32KiB, media_cap.max(MI_B)] — deliberately so a media cap above 1MiB (NVMe's 2MiB default) stays effective. The test only passed on machines detected as SSD/Unknown (cap == 1MiB) and failed on NVMe-backed CI runners with 2MiB != 1MiB. Assert the media cap itself, which is what the strategy actually guarantees on every environment.

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(storage): format buffer clamp assertion

* chore(logging): update tier guardrail path

---------

Signed-off-by: houseme <housemecn@gmail.com>
Co-authored-by: cxymds <cxymds@gmail.com>
Co-authored-by: overtrue <anzhengchao@gmail.com>
Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-03 18:11:14 +08:00