kvcache-ai / kvcache-ai/Mooncake
[Bug][Store]: Lease-expired client cleanup blocks all master RPC for minutes under HA + SSD Offload
- Dominant language
- C++
- Stars
- 6.6k
- Forks
- 1.2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 312
Description
### Bug Report
**Summary**
When a store's lease expires (client process stalls but doesn't crash — misses pings until lease TTL), the master's `ClientMonitorThreadFunc` synchronously runs `ClearInvalidHandles` → `ClearStaleHandles`: an O(total keys) full sweep of all shards, holding each shard's write lock. Because the sweep visits **every** key (not just the expired client's), cleaning a client that owns a large replica set (tens of millions, a fraction of total keys) takes minutes. During that time **all** RPCs — including `Ping` and `FetchTasks` — are blocked (`Total=0.00` in admin metrics), so the master appears hung even though the process is alive and `view_version` is unchanged.
> Reproduced on a deployment with SSD Offload enabled and **HA on, OpLog off**. So the bottleneck below is **not** OpLog/etcd throughput — it holds with pure in-memory deletion.
**Symptoms (observed in production)**
- ~7-minute "unresponsive" window (10:24:25 → 10:31:20).
- Admin metrics keep printing every 10s, `role=leader`, `view_version` unchanged (no HA re-election) → the master process is alive.
- During the window `PutStart/Get/Ping/FetchTasks` all show `Total=0.00`; `Clients` 8→7→8 (self-recovered at 10:31:46).
- `Mem Storage` pinned at 4.41 TB / 5.47 TB (80.7%) — unchanged, because the cleanup targets `local_disk`/SSD replicas, not memory.
- `SSD Storage` dropped 5.32 TB (37.47 → 32.15 TB); `Keys` dropped ~33.7M (234.1M → 200.5M).
- All requests from the inference engine (sglang) were blocked — get/put throughput was 0 for 7 minutes.
- All admin-metric counters (`Eviction/Discard/Promotion/Snapshot`) stayed 0 / unchanged, because `ClearStaleHandles` doesn't update any of them — making the work nearly invisible during diagnosis.
**Root cause analysis** — three compounding factors:
1. **The lease-expire path is unconditionally synchronous.** `ClientMonitorThreadFunc` calls `ClearInvalidHandles(alive_clients)` directly ([`master_service.cpp:11576`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_service.cpp#L11576)), unlike the normal unmount path which can go async via `replica_cleanup_worker_.Schedule()` ([`master_service.cpp:2858`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_service.cpp#L2858)). The lease-expire path doesn't even consult `enable_async_segment_cleanup_`.
2. **`ClearStaleHandles` holds each shard's write lock for the entire shard sweep** ([`master_service.cpp:2729-2798`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_service.cpp#L2729)). `MetadataShardAccessorRW shard(this, i)` scopes the whole shard loop; `PutStart`/`Get`/`Ping` all need a read lock on the shard → all blocked for the sweep duration.
3. **`ClearStaleHandles` is O(total keys), not O(stale keys).** The sweep iterates **every** key in **every** shard/tenant and calls `BuildStaleHandleCleanupPlan` on each ([`master_service.cpp:2738`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_service.cpp#L2738)), which walks that key's replicas and runs `is_stale` — and `has_stale_local_disk_client` does an `alive_clients.find()` hash lookup per replica ([`replica.h:465`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/include/replica.h#L465)). So even though only ~33.7M of 234M keys were stale, **all 234M keys are visited and judged**. Stale keys then go through `EraseMetadata` ([`master_service.cpp:2252`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_service.cpp#L2252)), which per key does several hash lookups/erasures (offloading_tasks, processing_keys, replication_tasks, soft-pin index). The whole pass is a large, cache-miss-bound in-memory traversal. This is independent of OpLog: with `enable_oplog_=false` the deletion path is pure in-memory `EraseMetadata` (no etcd write), yet the sweep still took ~7 min. OpLog, when enabled, adds a per-replica `Reserve`+`Commit` on top ([`master_service.cpp:2205-2226`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_service.cpp#L2205)); `Commit` is non-blocking but `Reserve` returns `TASK_PENDING_LIMIT_EXCEEDED` at 1024 in-flight slots ([`ordered_oplog_writer.cpp:189-191`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/ha/oplog/ordered_oplog_writer.cpp#L189)), which can only make things slower.
**Proposed approaches**
- **A:** Make `ClearStaleHandles` release the shard write lock periodically (time-sliced, e.g. every ~50ms) and resume from a key cursor (`lower_bound(last_key)`). While the lock is released, other RPCs may insert, delete, or modify keys in that shard — on resume, those keys are re-judged against `is_stale`: newly inserted keys don't belong to the expired client and are skipped, already-deleted keys are skipped by `lower_bound`, so releasing the lock mid-sweep is consistent. When OpLog is enabled, `ClearStaleHandles` only enqueues the OpLog entry + `mark_removed` and the actual `EraseMetadata` runs in the durable callback, which likewise supports releasing the lock mid-sweep. This keeps the master responsive regardless of cleanup volume — the "hung" symptom is fixed even if cleanup still takes minutes.
- **B:** Maintain a client→replica reverse index so lease-expire cleanup scans only that client's replicas (O(its replicas) vs O(total keys)). This directly attacks factor 3 — turning a 234M-key sweep into a 33.7M-key one. Larger change; suggest as a follow-up.
- **C (only when OpLog is enabled):** Increase `max_entries_per_batch` (1024 → larger) and/or batch `ClearStaleHandles`' OpLog submissions. Doesn't apply when OpLog is off (as in this report); when on, combine with A.
**Discussion**
- Is there a correctness reason `enable_async_segment_cleanup_` is disabled under HA, or is it conservative? If it can be lifted, the lease-expire path could use `replica_cleanup_worker_` directly.
- Which approach is preferred? Or if there's a better one, please share.
- Any concerns with the time-sliced lock release in `ClearStaleHandles` (cursor validity, intermediate consistency)?
### Before submitting...
- [ ] Ensure you searched for relevant issues and read the [documentation]
Contributor guide
Research direction
Start with ClientMonitorThreadFunc and ClearStaleHandles in mooncake-store/src/master_service.cpp, then read BuildStaleHandleCleanupPlan, EraseMetadata, and has_stale_local_disk_client in replica.h. Compare the lease-expire path at master_service.cpp:11576 with the normal unmount cleanup at :2858 and examine the locking around :2729-2798. Done means lease-expiry cleanup no longer blocks Ping, FetchTasks, PutStart, or Get for minutes while stale handles are removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, distributed-systems, performance
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100