[RFC]: Mooncake Store RPC correctness, compatibility, and execution model

## Background

Mooncake Store uses `coro_rpc` for two major communication paths:

- `MasterClient` ↔ `WrappedMasterService`
- `DummyClient` ↔ `RealClient`

An audit found one current correctness problem and several protocol compatibility and availability risks.

This issue records and prioritizes the current problems. It intentionally does not propose a concrete redesign; the expected compatibility guarantees and implementation direction should be discussed with the community first.

## Priority summary

| Priority | Finding | Impact |
|---|---|---|
| P0 | Client/server handler registries have already diverged | Existing RPC calls fail with “function not registered” |
| P1 | The wire contract is implicit and not safely evolvable | Rolling client/server upgrades cannot be guaranteed |
| P1 | Release-version equality is used as protocol negotiation | Both false rejection and false acceptance are possible |
| P1 | Blocking handlers occupy `coro_rpc` I/O threads | Slow requests can delay ping, readiness, and unrelated RPCs |
| P1 | Timed-out mutations have ambiguous outcomes | Clients cannot safely determine whether to retry |
| P2 | Error semantics and connection-state handling are inconsistent | Failures are difficult to classify and recover from |
| P2 | Request sizes and exposed RPC operations are insufficiently bounded | Resource-exhaustion and deployment-security risks |
| P2 | Platform-dependent C++ types are exposed on the wire | Compatibility depends on architecture and source layout |
| P3 | Large payload copies, incomplete metrics, and missing contract tests | Performance and regressions are difficult to quantify |

## P0: Confirmed functional failures

### Client methods without registered server handlers

The following methods are invoked by clients but are absent from the corresponding server registration lists:

- `WrappedMasterService::CalcCacheStats`
- `RealClient::batch_get_query_results`
- `RealClient::batch_get_replica_desc`
- `RealClient::get_replica_desc`

Relevant code:

- [`MasterClient::CalcCacheStats`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_client.cpp#L474)
- [Master RPC registration](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/rpc_service.cpp#L1637)
- [DummyClient `batch_get_query_results` call](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/dummy_client.cpp#L1122)
- [DummyClient replica descriptor calls](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/dummy_client.cpp#L1327)
- [RealClient RPC registration](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/real_client_main.cpp#L34)

These paths can currently fail with a “function not registered” error.

This is also direct evidence that maintaining the client call list, server registration list, and metrics metadata separately is already causing drift.

## P1: Protocol compatibility blockers

### The RPC contract is coupled to implementation classes

Endpoints are defined directly as members of `WrappedMasterService` and `RealClient`, while request and response payloads reuse internal business types.

Example: [`WrappedMasterService`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/include/rpc_service.h#L22)

Changes to implementation classes and internal data structures can therefore change the network protocol.

### Method IDs depend on C++ qualified names

The vendored `coro_rpc` implementation derives a 32-bit method ID from the qualified C++ function name.

This creates two different failure modes:

- Renaming a class, namespace, or method changes the method ID.
- Changing parameters or return types can keep the same method ID while changing the wire layout.

Mooncake currently has no explicit, reviewable list of stable RPC method IDs.

### Existing schema changes are not forward/backward compatible

Examples include:

- adding ordinary positional parameters such as `tenant_id`;
- adding `std::optional object_checksum` to `GetReplicaListResponse`;
- exposing `std::variant`-based replica descriptors directly over RPC.

Current response definition: [`GetReplicaListResponse`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/include/rpc_types.h#L40)

`std::optional` expresses optional application data but does not provide the schema-evolution behavior of `struct_pack::compatible`.

No `struct_pack::compatible` fields were found in Store RPC request or response types.

### Release version equality is used as protocol compatibility

[`MasterClient::Connect`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_client.cpp#L421) requires the client and server Mooncake Store version strings to be exactly equal.

This can:

- reject a compatible client/server pair after a release-version bump;
- accept an incompatible pair when both builds still report the same version.

The DummyClient ↔ RealClient readiness RPC does not exchange protocol version or capability information at all.

Together, these issues mean that rolling upgrades are not currently well-defined or mechanically verified.

## P1: Availability and execution-model risks

### Most server handlers are synchronous

In the audited Store RPC paths:

- approximately 60 Master handlers are registered, and none returns `async_simple::coro::Lazy`;
- approximately 45 RealClient handlers are registered, and only two are coroutine handlers:
- `batch_get_into_dummy_helper`;
- `batch_get_offload_object`.

Potentially blocking operations therefore execute synchronously in the RPC server path. These operations include Master RPC calls, SSD I/O, transfer waits, large batch loops, regex/full-state scans, and HA persistence work.

### Slow handlers can block ping and unrelated RPCs

The [standalone RealClient RPC server defaults to one thread](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/real_client_main.cpp#L22).

Two existing handlers explicitly move blocking work off the RPC I/O thread to keep ping responsive:

- [`batch_get_into_dummy_helper`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/real_client.cpp#L4481)
- [`batch_get_offload_object`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/real_client.cpp#L6520)

This protection is not consistently applied to the other potentially blocking handlers.

The current implementation can therefore exhibit head-of-line blocking: one slow request may delay ping, readiness checks, and unrelated client operations.

### Client call chains remain blocking

MasterClient and DummyClient create coroutine operations internally but immediately call `syncAwait`:

- [MasterClient invocation](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/master_client.cpp#L325)
- [DummyClient invocation](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/dummy_client.cpp#L191)

The network layer is asynchronous, but the surrounding Store call chains still consume a caller thread while waiting.

### Timed-out mutations have ambiguous outcomes

RPC timeout is currently a client-side wait timeout. There is no application-level cancellation or request deduplication identity.

When a mutating RPC times out:

- the server may continue executing it;
- the operation may commit after the client has returned an error;
- the client cannot reliably determine whether retrying is safe.

This affects operations such as put completion, remove, copy, move, mount, and unmount.

## P2: Reliability and robustness problems

### RPC errors are mapped inconsistently

MasterClient distinguishes `RPC_TIMEOUT` from `RPC_FAIL`.

DummyClient and `ClientRequester` generally map timeout, connection, and other transport failures to `RPC_FAIL`: [DummyClient error mapping](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/src/dummy_client.cpp#L203).

Callers therefore receive different error semantics depending on which Store RPC path they use.

### Connection-state edge cases are not fully guarded

Observed cases include:

- MasterClient methods can dereference an empty client pool when called before `Connect`;
- DummyClient may retain `connected_ == true` after switching to another address fails;
- some methods allowed while disconnected may still access an empty pool.

### Request and batch sizes are not uniformly bounded

No common Store-level limit was found for:

- RPC body size;
- attachment size;
- key length;
- number of keys per batch;
- total batch payload size.

The vendored RPC protocol allocates its receive buffer using the body length from the request header before Store-level validation.

Large or malformed requests can therefore cause high memory consumption or occupy an RPC thread for a long time.

### The default Master RPC listener is broadly exposed

The [default configuration listens on `0.0.0.0`](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/conf/master.yaml#L3-L8).

The audited startup paths do not enable RPC authentication or TLS, while the RPC surface includes mutating operations such as remove, mount, copy, and move.

The practical severity depends on the deployment network boundary.

### Platform-dependent types are present on the wire

Current signatures and payloads expose:

- `long`;
- `size_t`;
- `uintptr_t`;
- enums without a fixed underlying type;
- `std::variant`, whose meaning depends on alternative order.

Examples:

- [`long` in RPC signatures](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/include/rpc_service.h#L143)
- [`uintptr_t` and `size_t` in segment payloads](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/include/types.h#L438)
- [`std::variant` in replica descriptors](https://github.com/kvcache-ai/Mooncake/blob/main/mooncake-store/include/replica.h#L490)

This makes compatibility dependent on architecture, compiler configuration, and source-level ordering.

## P3: Performance, observability, and test gaps

### Large DummyClient payloads use the normal serialized body

DummyClient put/upsert methods pass `std::span` as regular RPC parameters. Large values therefore enter the `struct_pack` body and incur additional buffer allocation and copying.

The production impact has not yet been measured.

### RPC metrics cover only part of the lifecycle

Current metrics primarily record request count and successful request latency.

They do not consistently expose:

- failure and timeout latency;
- connection establishment time;
- connection-pool wait time;
- server queue wait time;
- in-flight requests;
- request and response size;
- detailed transport error categories.

### Batch failures can amplify logging

Several batch handlers log failures per key while executing synchronously. A large failing batch can generate significant logging work and delay the RPC worker further.

### Contract-level tests are missing

Current tests do not enforce:

- every client method has a registered server handler;
- old client ↔ new server compatibility;
- new client ↔ old server compatibility;
- stable method IDs;
- golden serialized payloads;
- ping responsiveness while a slow handler is running;
- maximum request and batch sizes.

For example, the existing `CalcCacheStats` test exercises the business object directly and does not detect its missing RPC registration.

## Findings that still require measurement

The following are plausible performance problems, but should not be treated as confirmed bottlenecks without measurements:

- I/O-thread starvation impact on ping and request p99;
- caller-thread consumption caused by `syncAwait`;
- serialization and copying cost for large DummyClient values;
- connection-pool reuse and waiting behavior;
- lock contention in Master batch operations;
- whether the configured Master RPC thread count hides or moves the bottleneck.

## Discussion scope

Before discussing a concrete design, the community should first agree on:

1. Whether Mooncake Store must support rolling client/server upgrades.
2. Which RPC paths are included in that compatibility guarantee.
3. How long different protocol versions are expected to coexist.
4. Whether the P0 registration failures should be fixed and tracked separately.
5. Which workloads should be used to quantify the P1 execution-model risks.

### Before submitting a new issue...

- [x] Searched existing issues for Store RPC compatibility, handler registration, and coroutine usage.
- [x] Read the relevant repository documentation and current implementation.

Start by reading mooncake-store/include/rpc_service.h and rpc_types.h, then trace the client and registration paths in src/master_client.cpp, dummy_client.cpp, rpc_service.cpp, real_client.cpp, and real_client_main.cpp. Review the existing CalcCacheStats test and the listed compatibility and responsiveness gaps. Done requires an agreed protocol and execution-model direction before implementation begins.

Written by the indexing model from the issue text.