kvcache-ai / kvcache-ai/Mooncake

[Performance]: BatchEvict full-metadata scan dominates eviction-cycle time at 1M objects (and holds snapshot_mutex_ for that duration)

Open
#2,560 9 comments 0 reactions 1 assignee Claimed by @yokinoshitayoki View on GitHub
Dominant language
C++
Stars
6.6k
Forks
1.2k
Avg merge
3d 5h
Merged PRs (30d)
312

Description

### Describe the issue

While reading `MasterService::BatchEvict` (`mooncake-store/src/master_service.cpp`), I wrote a small gtest benchmark that drives the real `BatchEvict` path with benchmark-only timing, to see where its time goes at large metadata scale. Filing the measurements to confirm whether this is worth addressing before proposing anything.

The candidate-selection path traverses all `kNumShards` × tenants × metadata each cycle, builds transient `candidates` vectors of `lease_timeout`, and runs `std::nth_element` to find the threshold — all under `snapshot_mutex_` held as a shared lock for the duration of the scan.

**All numbers are from a synthetic, single-tenant, all-expired, all-no-pin workload** (every object evictable: one completed memory replica, refcnt 0). The first pass reaches the eviction target, so the second pass is not exercised. Real workloads with soft-pinned / unexpired / multi-tenant objects will differ — I'm reporting the structural cost, not a production latency claim.

## Measurements

Commit `ef0312f8`, 128-core host, single-threaded `BatchEvict`, `evict_ratio_target=0.5`, `evict_ratio_lowerbound=0.25`.

### 1. End-to-end runtime is linear in object count

Bare (non-instrumented) `BatchEvict`:

| objects | runtime |
|---------|---------|
| 10k | ~8.7 ms |
| 100k | ~109 ms |
| 1M | ~0.95–1.0 s (median; n=5 range 930 ms–1.2 s, shared instance) |

### 2. The time is in scanning, not sorting or evicting

Per-phase breakdown at 1M (instrumented; ratios are the point, absolute total is higher due to instrumentation overhead):

| phase | share of cycle |
|-------|----------------|
| metadata scan / traversal | ~73–75% |
| └ candidate vector collection | ~28–31% |
| `std::nth_element` (all calls) | ~0.1% |
| actual eviction (`try_evict_group_or_object`) | <0.5% |

To be clear, I checked whether `nth_element` was the cost — it is not (~0.1%). The dominant cost is the full-metadata traversal to collect candidates.

### 3. The scan holds `snapshot_mutex_`, blocking unique-lock waiters

A probe thread issues a single `unique_lock` on `snapshot_mutex_` shortly after a cycle begins and records the wait. 30 trials at 1M:

| unique-lock wait | value |
|------------------|-------|
| p50 | ~1.00 s |
| p95 | ~1.21 s |
| max | ~1.36 s |

Wait p50 ≈ (cycle time − probe start delay), i.e. the waiter is blocked for essentially the whole scan. Because the shared lock is held once for the whole scan rather than re-acquired, the waiter did not measurably change `BatchEvict`'s own runtime in this probe.

## Question

Is the full-metadata scan in candidate selection considered acceptable at this scale, or would a candidate-selection structure that avoids repeated full scans (e.g. a lease-timeout-ordered auxiliary structure / per-shard expiration structure, in a similar spirit to the bounded-scan concerns raised for master-side promotion in #2509) be of interest?

I'm aware #1998 introduces radix-tree-based eviction for RadixAttention prefix-chain correctness. This report is meant to be complementary and narrower: the scan cost above exists independently of prefix semantics — it is purely about how candidates are found by `lease_timeout`. The two seem orthogonal and could land independently, but happy to be corrected if #1998 already subsumes this.

If reducing the scan / lock-hold time is of interest, I'm happy to put up a small PR — wanted to confirm the direction first.

## Reproduction

The benchmark drives the real path (`MountSegment` → `PutStart` → `PutEnd(..., ReplicaType::MEMORY)` → `BatchEvict`) with test-only timing through the existing friend-test pattern; it changes no production behavior. Happy to share the patch.

## Environment

- Building `main` near commit `ef0312f8`, store + transfer engine, TCP transport, RelWithDebInfo.
- Synthetic benchmark, single-threaded `BatchEvict`; not a production runtime failure.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.