[GH-ISSUE #4489] [RFC]: Object identity is unpinned between transfer submission and payload issue; wrong-object bytes can reach the wire despite 4/4 lease checks #1139

New issue

Open

opened 2026-10-29 21:23:15 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented 2026-10-29 21:23:15 +00:00

Owner

Copy link

Originally created by @binxuwang2002-lang on GitHub (Oct 7, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4489

Changes proposed

Summary

A transfer does deliver the bytes of an object other than the one named in its
descriptor into the requester's registered buffer, and reports the read as a
failure while doing so. A regression test attached below reproduces it
deterministically. The root cause is that object
identity is captured when the descriptor is created and consumed when the payload
is issued, with nothing holding it in force across the gap. The post-transfer
lease check protects the consumer from being told the read succeeded; it cannot
protect the wire, because the check runs after the bytes have already been
written into the destination buffer and discarding a result does not erase that
buffer. This issue asks whether a transfer-scoped hold on object identity belongs
in Mooncake Store, and if so, at which layer.

Setup and scope

Snapshot: c3fa13ecaf1038df933dcb784ff8ffe026381100
(2026-09-30T02:40:11Z). The tree we worked from is a plain source snapshot
without .git, so the anchor was established by per-file git blob SHA-1 against
the GitHub Contents API rather than by a git ref. Two files under test
(mooncake-store/src/client_service.cpp, mooncake-store/src/master_service.cpp)
match that commit exactly. The rest of the tree was not verified, so treat this
as a pin on those two files rather than on the repository state. main has
advanced slightly since (client_service.cpp is 6 lines longer as of 2026-10-04).
All IsLeaseExpired counts and line numbers below are from the pinned snapshot.

Configuration: memory replicas, the transfer engine, single node with two
clients. The trust model is a shared pool with multiple tenants, so isolation
between tenants is in scope. The repro below runs to completion on this snapshot;
its build and test logs are in the attached verification record.

Observation A: deterministic repro

The repro is 0002-repro-test.patch, a regression test. Sequence:

1. Put object X (single replica, known byte pattern) into a segment that has
   been filled so that exactly one object-sized gap remains.
2. BatchQuery obtains X's QueryResult while its lease is live.
3. Before issuing any transfer, evict X and put object Y so that Y
   reuses the same slot. Y carries a different byte pattern.
4. Issue BatchGet using the QueryResult from step 2.

Two things hold in the same run, and that is the point:

• Assertion A. The result is reported as a failure
  (LEASE_EXPIRED). The post-transfer lease check does its job.
• Assertion B. The destination buffer contains Y's bytes and does not
  contain X's. The bytes of a different object were delivered.

Assertion A is the property the existing post-transfer check provides. Assertion
B is the property no check at this layer can provide after the fact, because the
check inspects the result and discarding a result does not undo what was written
into the destination buffer.

The reuse this test produces is the same shape as the one the natural path
produces. In production the lease deadline passes, the ordinary eviction path
reclaims the slot, and the next Put lands on it. The test only removes the timing
dependence: it pins the placement to the segment the descriptor names, it calls
Remove(key, force) to reclaim without waiting out the clock, and it asserts as a
precondition that the replacement object really did land on the vacated slot,
endpoint and offset, before issuing the transfer. A run in which that
precondition fails never exercises the reuse and is not counted. The test passes
in isolation and inside the full client_integration_test suite, so the result
does not depend on run order.

What #3996 already covers, and what it does not

PR #3996 ("Check read leases after grouped batch gets", merged 2026-09-11) added
the post-transfer lease check to Client::BatchGetWhenPreferSameNode, which is
the fourth and last read path that was missing one. That was a real gap and it is
now closed: on the pinned snapshot IsLeaseExpired appears on all four read
paths, two in client_service.cpp at 1436 and 1481 (whole-object and ranged
gets) and two at 1650 and 1902 (grouped batch gets and BatchGet).

Two properties are worth separating, because they are not the same property:

1. The consumer is not reported a success. A read whose lease expired during
   the transfer is reported LEASE_EXPIRED. This is covered. The 2026 lease
   fixes and #3996 together cover all four read paths.
2. Wrong-object bytes are not delivered. The bytes written into the
   requester's registered buffer are the bytes of the object the descriptor
   names. This is not covered, and it cannot be covered by a check placed after
   the transfer, because the check inspects the result and discarding a result
   does not undo what was written into the buffer.

#3996's own regression test shows how far the reporting layer reaches, and it is
worth being precise about what it does and does not establish.
BatchGetChecksLeasesWithPreferredNode puts three keys that all carry the same
byte pattern, hands BatchGet a QueryResult whose deadline was set in the past
for one of them, and asserts
EXPECT_EQ(memcmp(expired_dst, value.data(), value.size()), 0) — the destination
buffer for that key holds value, which is the object's own content. There is no
eviction in that test and no other object takes the slot; the object is still
there. So it establishes that an expired lease does not stop the transfer from
completing and writing into the destination buffer. It does not establish that
the bytes delivered belong to a different object, because in that test they do
not.

The step from "the transfer completes anyway" to "the bytes belong to someone
else" needs a reuse to happen inside the window, and that is what the regression
test attached below adds. On the pinned snapshot that step is reachable: with the
slot reused while the descriptor is queued, the destination buffer receives the
replacement object's bytes. So the two tests are complementary. #3996 covers the
first step and does it well; the attached test covers the second.

The attached test also states both facts in one run: the read is reported as
a failure, so the reporting layer works, and the destination buffer holds another
object's bytes, so the reporting layer is not what protects the wire. Our own
test comment is explicit that this is the increment, not a re-statement of
#3996's.

On the pinned snapshot there are zero transfer-scoped holds in
client_service.cpp and master_service.cpp, and zero validations at payload
issue. Admission grants a lease; completion checks a lease; the moment the payload
is issued is unguarded.

Observation B: workload statistics

Three runs, two snapshots. B.1 and B.2 come from different snapshots, and B.3 adds
a third run, on current main. Each figure carries its own denominator
and convention. They are not comparable to each other and no ratio between
them should be read as an effect size: one is per observed eviction, one is per
completed transfer, one is per guard firing, one is per successful delivery under
a deliberately disabled guard. Each row stands alone.

B.1 Natural load, 2026-07 snapshot

Snapshot f20b7061097e4e2fda825f4106f215c71f13274a. Harness described in
0003-measurement-bypass-EXPERIMENTAL.patch. The guard is the shipped one.

quantity	value	interval	denominator and convention
race windows	36	—	out of 41,722 per-key evictions observed by the master census
window rate, per eviction	0.863 per 1000 evictions	Wilson 95% [0.623, 1.194]	36 / 41,722
window rate, per completion	6.08 per 1000 completions	Wilson 95% [4.40, 8.41]	36 / 5,918 completed transfers
guard firings	32,908	—	post-transfer lease check returned LEASE_EXPIRED
delivered wrong-object events	six observed events in which bytes of an object different from the one named in the descriptor were delivered into the requester's registered buffer	—	count only; no rate denominator was retained for this run

The six delivered events are reported as a count because no delivery denominator
survived for that run. They establish that conversion happens; they do not
establish how often.

B.2 Constructed burst, current main, guard shipped

Snapshot c3fa13ecaf1038df933dcb784ff8ffe026381100. 30 minutes, BurstGPT replay,
16 tenants, 32x oversubscription, tc 800mbit on loopback, and a probe injecting
2,000 ms between the descriptor capture and the transfer so the window opens on
every request. 18,262 reads.

Two quantities, both measured per guard firing, and both from the same run:

quantity	value	denominator and convention
guard firings	791	requests whose post-transfer lease check returned LEASE_EXPIRED, counted in the probe process
of those, payload already written with the requested object's own identity	791	per fired request; head and tail key hash both equal key_hash(requested key), generation in the reseed-allowed pair

Per-event identity was checked against the requested key for all 791 firings, not
sampled. Three controls accompany that zero: the comparison reuses the harness's
own key_hash rather than a second implementation, so the two cannot drift
apart; 716 requests that returned not-found carried no payload at all, which
rules out "the buffer always has something in it"; and all 791 fired requests
name the requesting tenant's own objects.

So on current main, with the shipped guard, the payload lands before the check
791 times, and in every one of those cases what landed was the requested
object's own bytes at a reseeded generation. This is the same behaviour
BatchGetChecksLeasesWithPreferredNode asserts, repeated 791 times under load.

B.3 Conversion rate on current main: bounded, not observed

The current-main conversion rate is bounded and not observed, not zero and
not measured:

• 0 foreign deliveries in 791 firings, rule-of-three upper bound
  3/791 = 3.79 per 1000 firings
• existence of conversion is carried by two other things, and neither is a
  current-main load number: the six delivered events in B.1, which are from the
  2026-07 snapshot and labelled as such, and the deterministic repro in
  Observation A, which establishes current existence but says nothing about rate

That arm has been run once before, on the 2026-07 snapshot: 4 of 65 successful
deliveries carried the wrong object, Wilson 95% [24.19, 147.82] per 1000
deliveries. That figure is a snapshot figure and is not a current-main rate, but
it establishes that the arm measures a non-zero conversion when one exists, so a
zero on current main would be informative rather than merely absent.

On current main the bypass arm has now been run once (30 min, same
configuration as the 2026-07 arm):

quantity	value	interval	denominator and convention
delivered wrong-object deliveries, guard disabled	0	rule-of-three bound 0.379 per 1000 deliveries	0 of 7,910 successful deliveries with the guard disabled; 3,389 bypass-released transfers were byte-verified out of band and every one matched the requested object's head and tail key hash (0 foreign, rule-of-three bound 0.885 per 1000 released transfers)

B.4 Interpreting the three together

What the three runs jointly support is narrow and is worth stating narrowly. The
window opens under natural load on the 2026-07 snapshot at 6.08 per 1000
completions, and it opens on every request under the constructed burst on current
main. Payload lands before the check runs, on current main, 791 times out of 791
firings. Whether what lands is ever a different object is bounded at 3.79 per
1000 firings on current main and is directly observed only on the 2026-07
snapshot, where it happened six times. The consumer-side property, that a read
whose lease expired is not reported as a success, is not in question and is not
what this issue is about.

Root cause: the window

   Query / BatchQuery
     |  lease granted, replica descriptors returned
     v
   descriptor queued          <- transfer engine, asynchronous
     |
     |  <-- the window: identity was captured here and is trusted below
     |
   evict + slot reused         <- owner changes; the descriptor still names the old owner
     |
     v
   payload issued              <- no check at this point
     |
     v
   transfer completes
     |
     v
   post-transfer lease check   <- 2026 fix and #3996; discards the result

Two observations about the current state. Admission is guarded (lease granted).
Completion is guarded (post-transfer check on all four read paths after the 2026
fix and #3996). Issue is not guarded at all, and the eviction path holds no
transfer-scoped reference to a descriptor that has been submitted but whose
payload has not been issued. real_client.cpp carries five lease checks (4175,
4588, 4703, 6421, 6494), all on the admission and metadata-construction side;
client_service.cpp carries four (1436, 1481, 1650, 1902), all after transfer
completion. None of them sits between descriptor creation and payload issue.

Design options

(a) Master-side transfer-scoped hold. Extend the lifetime of the lease, or
introduce a hold alongside it, so that it spans from descriptor submission to
transfer-completion signal rather than expiring at a wall-clock deadline. Reclaim
of a slot is refused while a hold names it. Cost: metadata growth proportional to
in-flight transfers, and a completion signal path that the eviction scheduler has
to observe.

(b) Issue-time validation. The transfer engine or the endpoint re-checks the
object generation or an epoch before the payload goes out. Cost: a check on the
fast path, where the whole point of the current design is that the fast path does
no bookkeeping.

(c) Quarantine or delayed reuse. A reclaimed slot is not handed to a new
object until an upper bound has elapsed. Cost: capacity held idle, and the bound
has to be derived from transfer-time distributions.

The two goals should be discussed separately, because the options do not cover
them equally. Consumer correctness is what the post-transfer check provides
today and what #3996 completed across all four read paths. Wire isolation —
the guarantee that the bytes placed on the wire and into the destination buffer
belong to the object named by the descriptor — requires a guard at or before the
issue point. Option (a) achieves both. Option (b) achieves wire isolation without
holding reclaim. Option (c) reduces the exposure window without closing it.

We have an instrumentation harness and are happy to run maintainer-proposed
configurations.

Questions to maintainers

1. Is wire isolation an explicit goal of Mooncake Store, or is consumer-side
   correctness the contract?
2. Where is the trust-model boundary? If tenants are trusted not to read each
   other's keys, does that change whether this is in scope?
3. What cost is acceptable on the fast path? The options above trade metadata
   size, a fast-path check, and capacity against the width of the exposure
   window.

Attachments

• 0001-observability-logging.patch — log-only instrumentation on the transfer
  completion and eviction paths, default off, no behaviour change.
• 0002-repro-test.patch — the regression test from Observation A. Assertion A
  is the existing post-transfer check; assertion B is the delivery of wrong-object
  bytes. The schedule is forced through the public Remove(key, force) API, which
  documents itself as skipping the lease check, and the test asserts as a
  precondition that the replacement object landed on the vacated slot before it
  issues the transfer.
• 0003-measurement-bypass-EXPERIMENTAL.patch — a guard bypass used only to
  measure how often the window converts when nothing discards the result. The
  bypass counts and quarantines; bytes are inspected by an out-of-band checker
  and never reported as a successful read to the caller. Not for production,
  not intended to be merged as-is.

We are not proposing a fix in this issue. If the maintainers consider wire
isolation in scope, the next step would be to agree on which of the three options
carries the acceptable cost, and we would be glad to measure whichever is
preferred.

Before submitting a new issue...

• Make sure you already searched for relevant issues and read the documentation

Originally created by @binxuwang2002-lang on GitHub (Oct 7, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4489 ### Changes proposed ## Summary A transfer does deliver the bytes of an object other than the one named in its descriptor into the requester's registered buffer, and reports the read as a failure while doing so. A regression test attached below reproduces it deterministically. The root cause is that object identity is captured when the descriptor is created and consumed when the payload is issued, with nothing holding it in force across the gap. The post-transfer lease check protects the consumer from being told the read succeeded; it cannot protect the wire, because the check runs after the bytes have already been written into the destination buffer and discarding a result does not erase that buffer. This issue asks whether a transfer-scoped hold on object identity belongs in Mooncake Store, and if so, at which layer. ## Setup and scope Snapshot: `c3fa13ecaf1038df933dcb784ff8ffe026381100` (2026-09-30T02:40:11Z). The tree we worked from is a plain source snapshot without `.git`, so the anchor was established by per-file git blob SHA-1 against the GitHub Contents API rather than by a git ref. Two files under test (`mooncake-store/src/client_service.cpp`, `mooncake-store/src/master_service.cpp`) match that commit exactly. **The rest of the tree was not verified**, so treat this as a pin on those two files rather than on the repository state. `main` has advanced slightly since (`client_service.cpp` is 6 lines longer as of 2026-10-04). All `IsLeaseExpired` counts and line numbers below are from the pinned snapshot. Configuration: memory replicas, the transfer engine, single node with two clients. The trust model is a shared pool with multiple tenants, so isolation between tenants is in scope. The repro below runs to completion on this snapshot; its build and test logs are in the attached verification record. ## Observation A: deterministic repro The repro is `0002-repro-test.patch`, a regression test. Sequence: 1. Put object `X` (single replica, known byte pattern) into a segment that has been filled so that exactly one object-sized gap remains. 2. `BatchQuery` obtains `X`'s `QueryResult` while its lease is live. 3. **Before issuing any transfer**, evict `X` and put object `Y` so that `Y` reuses the same slot. `Y` carries a different byte pattern. 4. Issue `BatchGet` using the `QueryResult` from step 2. Two things hold in the same run, and that is the point: - **Assertion A.** The result is reported as a failure (`LEASE_EXPIRED`). The post-transfer lease check does its job. - **Assertion B.** The destination buffer contains `Y`'s bytes and does not contain `X`'s. The bytes of a different object were delivered. Assertion A is the property the existing post-transfer check provides. Assertion B is the property no check at this layer can provide after the fact, because the check inspects the result and discarding a result does not undo what was written into the destination buffer. The reuse this test produces is the same shape as the one the natural path produces. In production the lease deadline passes, the ordinary eviction path reclaims the slot, and the next Put lands on it. The test only removes the timing dependence: it pins the placement to the segment the descriptor names, it calls `Remove(key, force)` to reclaim without waiting out the clock, and it asserts as a precondition that the replacement object really did land on the vacated slot, endpoint and offset, before issuing the transfer. A run in which that precondition fails never exercises the reuse and is not counted. The test passes in isolation and inside the full `client_integration_test` suite, so the result does not depend on run order. ## What #3996 already covers, and what it does not PR #3996 ("Check read leases after grouped batch gets", merged 2026-09-11) added the post-transfer lease check to `Client::BatchGetWhenPreferSameNode`, which is the fourth and last read path that was missing one. That was a real gap and it is now closed: on the pinned snapshot `IsLeaseExpired` appears on all four read paths, two in `client_service.cpp` at 1436 and 1481 (whole-object and ranged gets) and two at 1650 and 1902 (grouped batch gets and `BatchGet`). Two properties are worth separating, because they are not the same property: 1. **The consumer is not reported a success.** A read whose lease expired during the transfer is reported `LEASE_EXPIRED`. This is covered. The 2026 lease fixes and #3996 together cover all four read paths. 2. **Wrong-object bytes are not delivered.** The bytes written into the requester's registered buffer are the bytes of the object the descriptor names. This is not covered, and it cannot be covered by a check placed after the transfer, because the check inspects the result and discarding a result does not undo what was written into the buffer. #3996's own regression test shows how far the reporting layer reaches, and it is worth being precise about what it does and does not establish. `BatchGetChecksLeasesWithPreferredNode` puts three keys that all carry the same byte pattern, hands `BatchGet` a `QueryResult` whose deadline was set in the past for one of them, and asserts `EXPECT_EQ(memcmp(expired_dst, value.data(), value.size()), 0)` — the destination buffer for that key holds `value`, which is the object's own content. There is no eviction in that test and no other object takes the slot; the object is still there. So it establishes that an expired lease does not stop the transfer from completing and writing into the destination buffer. It does not establish that the bytes delivered belong to a different object, because in that test they do not. The step from "the transfer completes anyway" to "the bytes belong to someone else" needs a reuse to happen inside the window, and that is what the regression test attached below adds. On the pinned snapshot that step is reachable: with the slot reused while the descriptor is queued, the destination buffer receives the replacement object's bytes. So the two tests are complementary. #3996 covers the first step and does it well; the attached test covers the second. The attached test also states both facts in one run: the read is reported as a failure, so the reporting layer works, and the destination buffer holds another object's bytes, so the reporting layer is not what protects the wire. Our own test comment is explicit that this is the increment, not a re-statement of #3996's. On the pinned snapshot there are zero transfer-scoped holds in `client_service.cpp` and `master_service.cpp`, and zero validations at payload issue. Admission grants a lease; completion checks a lease; the moment the payload is issued is unguarded. ## Observation B: workload statistics Three runs, two snapshots. B.1 and B.2 come from different snapshots, and B.3 adds a third run, on current main. Each figure carries its own denominator and convention. They are **not comparable to each other** and no ratio between them should be read as an effect size: one is per observed eviction, one is per completed transfer, one is per guard firing, one is per successful delivery under a deliberately disabled guard. Each row stands alone. ### B.1 Natural load, 2026-07 snapshot Snapshot `f20b7061097e4e2fda825f4106f215c71f13274a`. Harness described in `0003-measurement-bypass-EXPERIMENTAL.patch`. The guard is the shipped one. | quantity | value | interval | denominator and convention | |---|---|---|---| | race windows | 36 | — | out of 41,722 per-key evictions observed by the master census | | window rate, per eviction | 0.863 per 1000 evictions | Wilson 95% [0.623, 1.194] | 36 / 41,722 | | window rate, per completion | 6.08 per 1000 completions | Wilson 95% [4.40, 8.41] | 36 / 5,918 completed transfers | | guard firings | 32,908 | — | post-transfer lease check returned `LEASE_EXPIRED` | | delivered wrong-object events | six observed events in which bytes of an object different from the one named in the descriptor were delivered into the requester's registered buffer | — | count only; no rate denominator was retained for this run | The six delivered events are reported as a count because no delivery denominator survived for that run. They establish that conversion happens; they do not establish how often. ### B.2 Constructed burst, current main, guard shipped Snapshot `c3fa13ecaf1038df933dcb784ff8ffe026381100`. 30 minutes, BurstGPT replay, 16 tenants, 32x oversubscription, tc 800mbit on loopback, and a probe injecting 2,000 ms between the descriptor capture and the transfer so the window opens on every request. 18,262 reads. Two quantities, both measured per guard firing, and both from the same run: | quantity | value | denominator and convention | |---|---|---| | guard firings | 791 | requests whose post-transfer lease check returned `LEASE_EXPIRED`, counted in the probe process | | of those, payload already written with the requested object's own identity | **791** | per fired request; head and tail key hash both equal `key_hash(requested key)`, generation in the reseed-allowed pair | Per-event identity was checked against the requested key for all 791 firings, not sampled. Three controls accompany that zero: the comparison reuses the harness's own `key_hash` rather than a second implementation, so the two cannot drift apart; 716 requests that returned not-found carried no payload at all, which rules out "the buffer always has something in it"; and all 791 fired requests name the requesting tenant's own objects. So on current main, with the shipped guard, the payload lands before the check 791 times, and in every one of those cases what landed was the requested object's own bytes at a reseeded generation. This is the same behaviour `BatchGetChecksLeasesWithPreferredNode` asserts, repeated 791 times under load. ### B.3 Conversion rate on current main: bounded, not observed The current-main conversion rate is **bounded and not observed**, not zero and not measured: - 0 foreign deliveries in 791 firings, rule-of-three upper bound **3/791 = 3.79 per 1000 firings** - existence of conversion is carried by two other things, and neither is a current-main load number: the six delivered events in B.1, which are from the 2026-07 snapshot and labelled as such, and the deterministic repro in Observation A, which establishes current existence but says nothing about rate That arm has been run once before, on the 2026-07 snapshot: 4 of 65 successful deliveries carried the wrong object, Wilson 95% [24.19, 147.82] per 1000 deliveries. That figure is a snapshot figure and is not a current-main rate, but it establishes that the arm measures a non-zero conversion when one exists, so a zero on current main would be informative rather than merely absent. On current main the bypass arm has now been run once (30 min, same configuration as the 2026-07 arm): | quantity | value | interval | denominator and convention | |---|---|---|---| | delivered wrong-object deliveries, guard disabled | **0** | rule-of-three bound 0.379 per 1000 deliveries | 0 of 7,910 successful deliveries with the guard disabled; 3,389 bypass-released transfers were byte-verified out of band and every one matched the requested object's head and tail key hash (0 foreign, rule-of-three bound 0.885 per 1000 released transfers) | ### B.4 Interpreting the three together What the three runs jointly support is narrow and is worth stating narrowly. The window opens under natural load on the 2026-07 snapshot at 6.08 per 1000 completions, and it opens on every request under the constructed burst on current main. Payload lands before the check runs, on current main, 791 times out of 791 firings. Whether what lands is ever a different object is bounded at 3.79 per 1000 firings on current main and is directly observed only on the 2026-07 snapshot, where it happened six times. The consumer-side property, that a read whose lease expired is not reported as a success, is not in question and is not what this issue is about. ## Root cause: the window ``` Query / BatchQuery | lease granted, replica descriptors returned v descriptor queued <- transfer engine, asynchronous | | <-- the window: identity was captured here and is trusted below | evict + slot reused <- owner changes; the descriptor still names the old owner | v payload issued <- no check at this point | v transfer completes | v post-transfer lease check <- 2026 fix and #3996; discards the result ``` Two observations about the current state. Admission is guarded (lease granted). Completion is guarded (post-transfer check on all four read paths after the 2026 fix and #3996). Issue is not guarded at all, and the eviction path holds no transfer-scoped reference to a descriptor that has been submitted but whose payload has not been issued. `real_client.cpp` carries five lease checks (4175, 4588, 4703, 6421, 6494), all on the admission and metadata-construction side; `client_service.cpp` carries four (1436, 1481, 1650, 1902), all after transfer completion. None of them sits between descriptor creation and payload issue. ## Design options (a) **Master-side transfer-scoped hold.** Extend the lifetime of the lease, or introduce a hold alongside it, so that it spans from descriptor submission to transfer-completion signal rather than expiring at a wall-clock deadline. Reclaim of a slot is refused while a hold names it. Cost: metadata growth proportional to in-flight transfers, and a completion signal path that the eviction scheduler has to observe. (b) **Issue-time validation.** The transfer engine or the endpoint re-checks the object generation or an epoch before the payload goes out. Cost: a check on the fast path, where the whole point of the current design is that the fast path does no bookkeeping. (c) **Quarantine or delayed reuse.** A reclaimed slot is not handed to a new object until an upper bound has elapsed. Cost: capacity held idle, and the bound has to be derived from transfer-time distributions. The two goals should be discussed separately, because the options do not cover them equally. **Consumer correctness** is what the post-transfer check provides today and what #3996 completed across all four read paths. **Wire isolation** — the guarantee that the bytes placed on the wire and into the destination buffer belong to the object named by the descriptor — requires a guard at or before the issue point. Option (a) achieves both. Option (b) achieves wire isolation without holding reclaim. Option (c) reduces the exposure window without closing it. We have an instrumentation harness and are happy to run maintainer-proposed configurations. ## Questions to maintainers 1. Is wire isolation an explicit goal of Mooncake Store, or is consumer-side correctness the contract? 2. Where is the trust-model boundary? If tenants are trusted not to read each other's keys, does that change whether this is in scope? 3. What cost is acceptable on the fast path? The options above trade metadata size, a fast-path check, and capacity against the width of the exposure window. ## Attachments - `0001-observability-logging.patch` — log-only instrumentation on the transfer completion and eviction paths, default off, no behaviour change. - `0002-repro-test.patch` — the regression test from Observation A. Assertion A is the existing post-transfer check; assertion B is the delivery of wrong-object bytes. The schedule is forced through the public `Remove(key, force)` API, which documents itself as skipping the lease check, and the test asserts as a precondition that the replacement object landed on the vacated slot before it issues the transfer. - `0003-measurement-bypass-EXPERIMENTAL.patch` — a guard bypass used only to measure how often the window converts when nothing discards the result. The bypass counts and quarantines; bytes are inspected by an out-of-band checker and never reported as a successful read to the caller. **Not for production, not intended to be merged as-is.** We are not proposing a fix in this issue. If the maintainers consider wire isolation in scope, the next step would be to agree on which of the three options carries the acceptable cost, and we would be glad to measure whichever is preferred. ### Before submitting a new issue... - [x] Make sure you already searched for relevant issues and read the [documentation](https://kvcache-ai.github.io/Mooncake/)

bsakaguchi added the

RFC

label 2026-10-29 21:23:15 +00:00

bsakaguchi commented 2026-10-29 21:23:16 +00:00

Author

Owner

Copy link

@github-actions[bot] commented on GitHub (Oct 7, 2026):

Thanks for opening this issue, @binxuwang2002-lang!

Field	Value
Issue	#4489
GitHub user ID	232507676
Reporter	@binxuwang2002-lang

A maintainer will triage this when possible. To help us respond faster, please include:

• Mooncake version or commit SHA
• Environment (OS, CUDA/driver, RDMA stack if relevant)
• Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

<!-- gh-comment-id:6032081683 --> @github-actions[bot] commented on GitHub (Oct 7, 2026): Thanks for opening this issue, @binxuwang2002-lang! | Field | Value | |-------|-------| | **Issue** | #4489 | | **GitHub user ID** | `232507676` | | **Reporter** | @binxuwang2002-lang | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented 2026-10-29 21:23:16 +00:00

Author

Owner

Copy link

@he-yufeng commented on GitHub (Oct 7, 2026):

Confirmed the mechanism reading against current main (the report pins c3fa13ec; main has since moved client_service.cpp by 6 lines, and the picture is unchanged):

• The four IsLeaseExpired checks now sit at client_service.cpp:1442/1487/1656/1908, the same four the report lists at 1436/1481/1650/1902. All four run after the transfer has completed and the slices are filled, i.e. they inspect the result.
• Remove(key, force) does skip the lease check (master_service.cpp:7397), so the repro's scheduling primitive behaves as described.
• Nothing sits between descriptor capture at query time and payload issue. Every master-side check is on the admission or eviction path.

Two pieces of current-main context that matter for the design discussion:

1. All four read paths also run VerifyObjectChecksum post-transfer (1424, 1627, 1835, 1882), binding the delivered bytes to the object_checksum captured at admission. It is off by default (MOONCAKE_STORE_CHECKSUM, src/config/client_object_checksum_config.cpp:14). With checksums enabled, the repro's assertion A surfaces as CHECKSUM_MISMATCH before the lease check even runs, so an honest consumer is told the bytes are foreign. It still says nothing about what remains in the registered buffer afterwards, so the wire-isolation question stands exactly as the report frames it. The checksum just moves the honest-consumer property from timing to identity, when it is on.

2. The eviction-side half of option (a) already exists. hard_pinned metadata is honored at every reclaim point I can find (master_service.cpp:1813, 8350, 11511, 11969, 12075, 12264, 12318, 12485) and is preserved across metadata rebuilds (6114-6119). What option (a) actually needs to build is the lifecycle: take the hold at query or admission, release it on completion, and give it a TTL for the crashed-client case. That reframes the cost. Metadata growth is a counter per replica, not a list. The real costs are the completion-signal path and the semantics of the TTL firing.

On that TTL, one asymmetry is worth stating: a hold deadline that expires mid-transfer fails closed (reclaim refused, capacity pressure, visible in metrics), where the current lease deadline fails open (foreign bytes delivered, silent). If the hold ends up being "a second lease with different failure semantics", that is still a strict improvement in failure mode, and it makes option (a) cheaper to reason about than it first looks.

On the options themselves: (b) needs identity at the source before the RDMA read, which means a generation in the descriptor plus a source-side metadata read on the issue path. That is exactly the bookkeeping this fast path is designed to avoid, and the checksum binding already covers the honest-consumer side of identity from the other end. (c) spends capacity to shrink a window it cannot close. There is also an unlisted (d): transfer into a scratch buffer and copy into the destination only after the post-transfer checks pass. It closes the channel completely, at the cost of an extra memcpy and giving up true zero-copy on reads. For deployments where that bandwidth trade is acceptable it is the cheapest correct answer, and it is a useful cost reference when judging (a).

On the three questions: today's shipped contract is consumer correctness (the 2026 lease fixes, #3996, and the optional checksum). Whether wire isolation is an explicit goal is a maintainer-scope call, but the repro's segment-shaping precondition is precisely the primitive a tenant would use to pull another tenant's bytes on purpose, and the report demonstrates the schedule is controllable. In a shared-pool deployment that reads as a disclosure channel rather than a correctness nuance, so my lean is in scope, at minimum as a documented trust-model boundary. @stmatengss @doujiang24 this needs your call on the trust-model question. On fast-path cost, my ordering from the mechanism side is (a) if the completion signal is acceptable, then (d) where zero-copy is not the point, then (c) as mitigation.

Happy to review a design once there is a direction. The harness work here is solid; keeping the per-run denominators separate is what makes it convincing.

<!-- gh-comment-id:6033271559 --> @he-yufeng commented on GitHub (Oct 7, 2026): Confirmed the mechanism reading against current main (the report pins c3fa13ec; main has since moved `client_service.cpp` by 6 lines, and the picture is unchanged): - The four `IsLeaseExpired` checks now sit at `client_service.cpp:1442/1487/1656/1908`, the same four the report lists at 1436/1481/1650/1902. All four run after the transfer has completed and the slices are filled, i.e. they inspect the result. - `Remove(key, force)` does skip the lease check (`master_service.cpp:7397`), so the repro's scheduling primitive behaves as described. - Nothing sits between descriptor capture at query time and payload issue. Every master-side check is on the admission or eviction path. Two pieces of current-main context that matter for the design discussion: 1. All four read paths also run `VerifyObjectChecksum` post-transfer (1424, 1627, 1835, 1882), binding the delivered bytes to the `object_checksum` captured at admission. It is off by default (`MOONCAKE_STORE_CHECKSUM`, `src/config/client_object_checksum_config.cpp:14`). With checksums enabled, the repro's assertion A surfaces as `CHECKSUM_MISMATCH` before the lease check even runs, so an honest consumer is told the bytes are foreign. It still says nothing about what remains in the registered buffer afterwards, so the wire-isolation question stands exactly as the report frames it. The checksum just moves the honest-consumer property from timing to identity, when it is on. 2. The eviction-side half of option (a) already exists. `hard_pinned` metadata is honored at every reclaim point I can find (`master_service.cpp:1813, 8350, 11511, 11969, 12075, 12264, 12318, 12485`) and is preserved across metadata rebuilds (6114-6119). What option (a) actually needs to build is the lifecycle: take the hold at query or admission, release it on completion, and give it a TTL for the crashed-client case. That reframes the cost. Metadata growth is a counter per replica, not a list. The real costs are the completion-signal path and the semantics of the TTL firing. On that TTL, one asymmetry is worth stating: a hold deadline that expires mid-transfer fails closed (reclaim refused, capacity pressure, visible in metrics), where the current lease deadline fails open (foreign bytes delivered, silent). If the hold ends up being "a second lease with different failure semantics", that is still a strict improvement in failure mode, and it makes option (a) cheaper to reason about than it first looks. On the options themselves: (b) needs identity at the source before the RDMA read, which means a generation in the descriptor plus a source-side metadata read on the issue path. That is exactly the bookkeeping this fast path is designed to avoid, and the checksum binding already covers the honest-consumer side of identity from the other end. (c) spends capacity to shrink a window it cannot close. There is also an unlisted (d): transfer into a scratch buffer and copy into the destination only after the post-transfer checks pass. It closes the channel completely, at the cost of an extra memcpy and giving up true zero-copy on reads. For deployments where that bandwidth trade is acceptable it is the cheapest correct answer, and it is a useful cost reference when judging (a). On the three questions: today's shipped contract is consumer correctness (the 2026 lease fixes, #3996, and the optional checksum). Whether wire isolation is an explicit goal is a maintainer-scope call, but the repro's segment-shaping precondition is precisely the primitive a tenant would use to pull another tenant's bytes on purpose, and the report demonstrates the schedule is controllable. In a shared-pool deployment that reads as a disclosure channel rather than a correctness nuance, so my lean is in scope, at minimum as a documented trust-model boundary. @stmatengss @doujiang24 this needs your call on the trust-model question. On fast-path cost, my ordering from the mechanism side is (a) if the completion signal is acceptable, then (d) where zero-copy is not the point, then (c) as mitigation. Happy to review a design once there is a direction. The harness work here is solid; keeping the per-run denominators separate is what makes it convincing.

bsakaguchi commented 2026-10-29 21:23:17 +00:00

Author

Owner

Copy link

@binxuwang2002-lang commented on GitHub (Oct 8, 2026):

Thanks for the independent pass over current main. The +6 drift matches our pin exactly — same four sites, same ordering.

On the checksum: agreed on the mechanism, with one boundary worth keeping explicit. All our runs use the default configuration, i.e. checksum off. When it is on, a rebound slot surfaces as CHECKSUM_MISMATCH, which moves the honest-consumer property from timing to identity — that framing is right. But the check runs after the bytes land, and it says nothing about what remains in the registered buffer. So the two claims stay separable: "the consumer is told" holds when checksums are on; "the wire stays clean" is untouched either way. Worth stating plainly because the second claim is the one a shared-pool deployment actually cares about.

On hard_pinned: agreed, and this reframes option (a) usefully. If the metadata bit is already honored at the reclaim points and survives rebuilds, what is missing is purely the lifecycle — acquire at query, release at completion, TTL for the crashed-client case. Your fail-closed/fail-open asymmetry is the right way to put it: a hold that expires mid-transfer refuses reclaim and shows up in metrics, while a lease that expires mid-transfer delivers foreign bytes silently. Even "a second lease with different failure semantics" would be a strict improvement in failure mode.

On (d): fair addition, and the strongest of the four on the wire-isolation axis. The cost is regime-dependent though: the memcpy is bandwidth-bounded, for large transfers it approaches the cost of the transfer itself, the scratch pool consumes capacity that could otherwise hold objects, and the copy sits on the read's critical path. Where zero-copy is not the selling point, it is likely the cheapest correct answer — and a useful cost reference when judging (a), as you said.

One precision on our scope, since the trust-model question is now with the owners: we demonstrated a controllable schedule and wrong-bytes delivery in a shared-pool deployment (16 tenants). We have not built an end-to-end directed exfiltration PoC, and we don't want to overstate that step. The shared pool is the deployment shape we measured.

We stay in measurement mode. If there is a configuration or regime you would find more persuasive — checksum on, different tenant counts, larger objects — we are happy to run it and report numbers under whatever framing the owners settle on for the trust-model question.

<!-- gh-comment-id:6049320412 --> @binxuwang2002-lang commented on GitHub (Oct 8, 2026): Thanks for the independent pass over current main. The +6 drift matches our pin exactly — same four sites, same ordering. On the checksum: agreed on the mechanism, with one boundary worth keeping explicit. All our runs use the default configuration, i.e. checksum off. When it is on, a rebound slot surfaces as CHECKSUM_MISMATCH, which moves the honest-consumer property from timing to identity — that framing is right. But the check runs after the bytes land, and it says nothing about what remains in the registered buffer. So the two claims stay separable: "the consumer is told" holds when checksums are on; "the wire stays clean" is untouched either way. Worth stating plainly because the second claim is the one a shared-pool deployment actually cares about. On hard_pinned: agreed, and this reframes option (a) usefully. If the metadata bit is already honored at the reclaim points and survives rebuilds, what is missing is purely the lifecycle — acquire at query, release at completion, TTL for the crashed-client case. Your fail-closed/fail-open asymmetry is the right way to put it: a hold that expires mid-transfer refuses reclaim and shows up in metrics, while a lease that expires mid-transfer delivers foreign bytes silently. Even "a second lease with different failure semantics" would be a strict improvement in failure mode. On (d): fair addition, and the strongest of the four on the wire-isolation axis. The cost is regime-dependent though: the memcpy is bandwidth-bounded, for large transfers it approaches the cost of the transfer itself, the scratch pool consumes capacity that could otherwise hold objects, and the copy sits on the read's critical path. Where zero-copy is not the selling point, it is likely the cheapest correct answer — and a useful cost reference when judging (a), as you said. One precision on our scope, since the trust-model question is now with the owners: we demonstrated a controllable schedule and wrong-bytes delivery in a shared-pool deployment (16 tenants). We have not built an end-to-end directed exfiltration PoC, and we don't want to overstate that step. The shared pool is the deployment shape we measured. We stay in measurement mode. If there is a configuration or regime you would find more persuasive — checksum on, different tenant counts, larger objects — we are happy to run it and report numbers under whatever framing the owners settle on for the trust-model question.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

te-coro

staryxchen-tent-capability-research

feature/musa-ep-jit

zxpdemonio/fix-tent-bootstrap-admission

add-store-rs-tone-tests

scatter-rdma-completion-pr2

P2P-Mooncake-Store

release/v0.3.14

feat/tent-rdma-adaptive-adapter

fix/ep-local-counter-gpu-scope

codex/kvcs-oplog-eviction

give-me-a-star

cruz/mooncake-kvcs-lowlevel-pr1-20261008

ylt-handshake-limits

ci/rocm-e2e-tent

sima/docs-pypi-downloads

fix-ci/hicache-mooncake-metadata-loopback

sima/cuda-driver-loading

sima/ci-performance-gate

fix/tent-rdma-unknown-topology-rail-pairing

ci/tent-e2e-dual-job

mc-patch-1

codex/p2p-merge-prerequisites

sima/store-hca-numa-placement

codex/p2p-merge-cli

codex/p2p-merge-ipv6

copilot/fix-ctests-job-failure

czh/pg-device-collectives-pr6

dependabot/cargo/mooncake-store-rs/pyo3-0.29.0

czh/pg-device-collectives-pr5

codex/segment-pool-master-cutover

sima/nightly-ci-recovery

ci/tent-params

stmatengss-patch-1

dependabot/cargo/mooncake-store-rs/rand-0.8.8

dependabot/cargo/mooncake-store-rs/opentelemetry_sdk-0.32.1

dependabot/cargo/mooncake-store-rs/xxhash-rust-0.8.16

codex/migrate-store-rest-service

fix/tone-wheel-arch-selection

czh/pg-device-collectives-pr4

czh/pg-device-collectives-pr3

ketong/fix_moe_tc

copilot/fix-test-wheel-ubuntu-job

fix/ci-wheel-torch-pg-pin

codex/migrate-structured-object-storage

codex/migrate-allocator-python

codex/migrate-vllm-python

codex/migrate-shared-segment

ci/fix_sgl_eval_install

codex/segment-pool-nof-placement-cleanup

feat/pip-native-release

copilot/bugfix-ha-standby-promotion

copilot/mooncake-store-transport-settings

copilot/fix-tensor-support-issues

copilot/optimize-rl-direction

copilot/mooncake-storeagentic-workflow

copilot/fix-client-reconnect-timeout

copilot/mooncake-error-debugging

copilot/mooncake-ipv6-support

feat/nccl-host-python-api

dependabot/go_modules/mooncake-p2p-store/src/p2pstore/google.golang.org/grpc-1.82.1

copilot/migrate-and-move-operations

copilot/migrate-and-move-operations-again

refactor/centralize-env-vars

idhanani/mooncake-retain-groups

copilot/codeownerreviewer

copilot/give-feedback-on-issue-2123

copilot/create-issue-rfc

copilot/break-cuda-monopoly-logic

copilot/compare-cudamemcpy-cu12-cu13

copilot/report-ci-error-issue

copilot/research-commit-history-analysis

copilot/explain-mooncake-store-service

copilot/analyze-project-functionality

copilot/sub-pr-1680

copilot/review-pr-1680

copilot/sub-pr-1597

copilot/fix-github-actions-job-failure

copilot/move-test-sglang-integration-job

v0.3.14

v0.3.14-pre-4

v0.3.14-pre-3

v0.3.14-pre-2

v0.3.14-pre

v0.3.14-rc1

v0.3.13.post1

v0.3.13-rc2

v0.3.13-rc1

v0.3.13

v0.3.12.post1

v0.3.12

v0.3.11-rc1

v0.3.11.post1

v0.3.11

v0.3.10.post2

v0.3.10.post1

v0.3.10

v0.3.9

v0.3.8.post1

v0.3.8

v0.3.7.post2

v0.3.7.post1

v0.3.7

v0.3.6.post1

v0.3.6

v0.3.5

v0.3.4.post2

v0.3.4.post1

v0.3.4

v0.3.3.post2

v0.3.3.post1

v0.3.3

v0.3.2.post1

v0.3.2

v0.3.1

v0.3.0

v0.3.0-beta

v0.3.0-alpha

v0.2.0

Labels

Clear 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

Mirrored from GitHub Pull Request

  

PyTorch Backend

  

question

  

RFC

  

Roadmap

  

stale

  

Store

  

Tests

  

Transfer Engine

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

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

kvcache-ai/Mooncake#1139

No description provided.