mirror of
https://github.com/kvcache-ai/Mooncake.git
synced 2026-11-05 01:17:55 +00:00
[GH-ISSUE #2827] [Bug]: [SSD] DSV4 enable SSD offload，stress testing, repeat offloading the same batch key, lead to OBJECT_ALREADY_EXISTS/persist failed/INVALID_KEY #733
Labels
No labels
ascend
auto-closed
bug
bug
CI/Build
Common
documentation
enhancement
feature request
good first issue
help wanted
high priority
MetaX
nightly-failure
pull-request
PyTorch Backend
question
RFC
Roadmap
stale
Store
Tests
Transfer Engine
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
kvcache-ai/Mooncake#733
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @huangdong2022 on GitHub (Jul 10, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2827
Bug Report
测试场景：
DeepSeekV4模型，PD分离，4机，1P1D，P和D都是双机。
P节点的1台机器的vllm启动脚本。
mooncake_master的启动命令。
P节点的vllm日志
单条请求测试（50K输入，1K输出）时，mooncake + 池化 + ssd没有问题。
32并发，50K输入，1K输出时，会报错，报错见：deepseekv4_start_prefill_XX.log
分析
问题定位结论
32 并发下失败的核心链路是 Mooncake SSD Offload 并发写冲突 → 整批 persist 失败 → Master 与本地 SSD 元数据不一致 → D 侧读 KV 时
INVALID_KEY。单请求不触发，是因为没有并发重复 offload。1. 错误时间线（日志）
Duplicate key detected in BatchOffload→OBJECT_ALREADY_EXISTSKey not found→BatchQuerySlices/BatchLoad INVALID_KEYBatch get offload object failed, err_code = INVALID_KEY（约 4.7 万次）missing_count=N持续出现2. 根因分析
根因 A：并发 offload 同一 KV key（主因）
日志典型片段：
触发条件（32 并发 + 池化 + prefix caching）：
object_bucket_map_检测到 key 已存在，返回OBJECT_ALREADY_EXISTS本地代码路径（
storage_backend.cpp）：根因 B：
file_storage.cpp未将OBJECT_ALREADY_EXISTS视为幂等成功PutStart路径会把OBJECT_ALREADY_EXISTS当成功：但 Offload persist 路径遇到该错误会直接失败整批：
后果：
BatchGet→Key not found→INVALID_KEY根因 C：配置加剧并发冲突
enable_prefix_cachingrun_pd-p.sh写了--no-enable-prefix-caching，但实际运行未生效）default_kv_lease_ttlmax_num_seqsoffload_on_evict=true3. 错误传播链
4. 非主因（可忽略）
libjemalloc.so.2LD_PRELOAD 警告：环境缺库，不影响功能ValueError: ... is not a multimodal model：个别请求带了 image 内容，与 Mooncake KV 无关reasoning token IDs警告：DSV4 parser 配置问题，非本次根因5. 修复建议（按优先级）
P0 — Mooncake 代码修复
file_storage.cpp的OffloadObjects中，对OBJECT_ALREADY_EXISTS按幂等成功处理（与PutStart一致），不要 fail 整批。storage_backend.cpp的BatchOffload中，duplicate key 时 跳过该 key、继续提交其余 key，而不是 return error。P1 — 配置调整
P2 — 验证步骤
Failed to persist objects with error: OBJECT_ALREADY_EXISTSBatch get offload object failed, INVALID_KEY归零missing_count不再持续增长/opt/mooncake_offload/rank_*/下 bucket 文件与 Master 元数据一致6. 总结
OBJECT_ALREADY_EXISTS→INVALID_KEYfile_storage未做幂等处理，整批 persist 失败OBJECT_ALREADY_EXISTS幂等 + 增大 lease TTL + 确认关闭 prefix cachingBefore submitting...
@github-actions[bot] commented on GitHub (Jul 10, 2026):
Thanks for opening this issue, @huangdong2022!
161736910A maintainer will triage this when possible. To help us respond faster, please include:
Useful links: Documentation · Contributing guide
@he-yufeng commented on GitHub (Jul 13, 2026):
Triaging this one — I traced it through the SSD offload path and I'm fairly confident it's a distinct issue from the
INVALID_KEYrace in #2799 (which #2818 covers). TheOBJECT_ALREADY_EXISTS→ "persist failed" here is about how the offload orchestrator handles an already-offloaded key, and it sits in a different layer than #2818's change.Root cause. When the same batch/key gets offloaded again (which does happen under stress/retry), the key is already present in
object_bucket_map_, soBucketStorageBackend::BatchOffloadtrips its duplicate pre-check and returnsOBJECT_ALREADY_EXISTSfor the whole bucket. Back in the orchestrator (thestorage_backend_->BatchOffload(...)caller inFileStorage), the error handling only special-casesKEYS_ULTRA_LIMITandINVALID_READ; everything else — includingOBJECT_ALREADY_EXISTS— falls into theelseand is propagated as a hard failure for the entire offload (condensed):Contrast with the client Put path, which deliberately treats the same code as benign —
Client::BatchPuthas an explicit "if error == object already exist, consider as ok (Put semantics)" branch, and thePutStarthandler returns{}onOBJECT_ALREADY_EXISTS. The offload path doesn't apply that idempotency, so re-offloading an already-persisted key becomes a persist failure.Amplification. It's worse than a single key failing: the duplicate pre-check returns before any key in the bucket is committed, so one already-present key fails the whole bucket and takes the fresh keys grouped with it down too. There's also no dedup against
object_bucket_map_before grouping (GroupOffloadingKeysByBucketonly drops keys that don't fit into a bucket), so re-submitted keys always reach the pre-check.Fix direction — want to align on the semantics first. A few options, none a clean one-liner:
BatchOffloadskip already-present keys and commit the rest (partial success), reporting the skipped ones as already-persisted instead of failing the bucket. Most correct, but it changes the return contract.GroupOffloadingKeysByBucket, so they never enter a bucket and instead fall through the existing "skipped keys" path that reports viaNotifyOffloadSuccess.OBJECT_ALREADY_EXISTSas non-fatal — but on its own that's unsafe here, because the bucket's non-duplicate keys were never committed, so we can't call the batch a success.Option 2 reads cleanest to me — keep the "already persisted, nothing to do" decision out of the hot commit path — but it depends on what invariant we want
BatchOffloadto guarantee on a re-offload. I'm happy to put up the PR once we settle on the intended idempotency behavior. cc @LujhCoconut since this is adjacent to the #2799/#2818 work.@huangdong2022 — to confirm the diagnosis, could you share the commit SHA you hit this on, and (if you have them) the surrounding log lines where
OBJECT_ALREADY_EXISTSfirst appears before thepersist failed? That'll tell us whether the re-offload is coming from a retry after a partial failure or from the normal eviction/offload loop re-selecting the same keys.@huangdong2022 commented on GitHub (Jul 13, 2026):
@he-yufeng The supplementary information is as follows:
1、Mooncake version： commitid (main branch) ->
c9896684fb2、surrounding log lines where OBJECT_ALREADY_EXISTS first appears before the persist failed：
If there is anything else you would like to know, feel free to ask me anytime.
@LujhCoconut commented on GitHub (Jul 13, 2026):
@he-yufeng Good analysis. I agree Option 2 is the right semantics: re-offloading an already-persisted key should be a no-op, same as Put. If it's already on disk, Master should just be told so.
The one gap is TOCTOU —
GroupOffloadingKeysByBucket'sIsExistcheck andBatchOffload's pre-check run under different locks (offloading_mutex_vsmutex_), withBuildBucket+WriteBucketdisk I/O in between. A duplicate key committed by another thread during that window still lands in the pre-check, which still kills the bucket. Option 1's skip-instead-of-fail undermutex_closes that residual window.So they're not competing: Option 2 is the fast-path dedup, Option 1 is the correctness safety net. Agree with starting with Option 2 — it handles the vast majority of duplicates with clean semantics: already-persisted = success. The TOCTOU residual (between the two locks, across disk I/O) can be tightened later with Option 1's skip-instead-of-fail as a safety net.
@he-yufeng commented on GitHub (Jul 13, 2026):
Picked this back up and traced the offload path end to end on
main. Three things — two that narrow the fix, and one semantics question I'd like to settle before I open the PR.Option 2 is effectively already in place.
GroupOffloadingKeysByBucketalready skips already-persisted keys through itsIsExistbranch, so a plain re-offload of a persisted key never enters a bucket — it drops through theall_bucket_keysgap inOffloadObjectsand gets reported viaNotifyOffloadSuccesswith thedata_size=-1sentinel. The clean "already persisted = success" path is what the code already does; this issue is the set of cases that slip past it.Two ways a duplicate still reaches
BatchOffload's pre-check:IsExistruns underoffloading_mutex_, the commit runs undermutex_, withBuildBucket/WriteBucketin between; a key persisted by another offload in that gap passes grouping but trips the pre-check.ungrouped_offloading_objects_carry-over — parked keys are drained straight into the next bucket without going back through theIsExistbranch, so a key that got persisted via another bucket between rounds reaches the pre-check with no concurrency at all. That path just needs retries/backpressure, which lines up with the stress repro.The part that makes Option 1 more than flipping the pre-check.
complete_handler(→NotifyOffloadSuccess) runs before the duplicate pre-check that thenCleanupOrphanedBuckets the freshly written bucket. So on today's dup path the master is handed the whole bucket's keys and metadata for a bucket that's deleted a few lines later, and theelseinOffloadObjectsstill turns the return into a hard failure with no compensating revoke. Before I wire up skip-instead-of-fail I want the reporting to be right: it has to drivecomplete_handleroff the committed keys — report only what actually landed — rather than the fullbucket->keys. Do you know off-hand whether the heartbeat rescan reconciles master records against the actual bucket files, or would the master sit on metadata for a deleted bucket until the next scan? That's what decides report-before-commit vs commit-before-report for the skip path, and it's the one bit I don't want to guess at in a data path.I already have skip-instead-of-fail implemented with the store tests green locally (the
Duplicate*cases reworked to the idempotent contract). I'd rather settle the reporting semantics above than guess at the return-contract change — happy to push the PR the moment we're aligned.@LujhCoconut commented on GitHub (Jul 14, 2026):
@he-yufeng Traced the heartbeat/reconcile paths end-to-end against the code. Short answer: no reconcile. Phantom
metadata persists in steady state.
Here's why:
OffloadObjectHeartbeattakes(client_id, bool)and returns a list of offload tasks — it's the master telling the client what to offload, not the client telling the master what's on disk.ReRegisterOffloadedObjectsis additive-only. It walks the in-memorybuckets_map and callsNotifyOffloadSuccessfor each key. It never prunes stale master records. And it only fires onSEGMENT_NOT_FOUND(master restart), never periodically.Init()is purely local. Scanning.metafiles, loading into memory, cleaning orphaned.bucket— all local, zero RPC to master.CleanupStaleHandleschecks liveness, not file existence. The only master-side cleanup of LOCAL_DISK replicas fires when the client process dies, not when a bucket file is missing.So a phantom replica (reported before
CleanupOrphanedBucketdeleted the files) sits on the master forever as long as the worker stays alive. Commit-before-report is mandatory. Your skip-instead-of-fail approach is the right fix — go ahead with the PR, happy to review.