kvcache-ai / kvcache-ai/Mooncake

[RFC]: Master rolling upgrade has no index-preserving path today, and no readiness signal predicts a safe promotion

Open
#3,774 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
6.6k
Forks
1.2k
Avg merge
3d 5h
Merged PRs (30d)
312

Description

## Summary

There is currently **no upgrade path for the Master that preserves the cache index** — not rolling, not stop-and-restore. We measured all three available paths on two clusters running `0.3.13` and each one loses the index, by three different mechanisms.

This is filed as a new RFC rather than a comment on #1920 because #1920 is closed and, more importantly, because its **"Current Capabilities" section describes a baseline that does not hold in `0.3.13`**. Three of the capabilities it builds on are broken, and one of them makes #1920's own proposed Master Upgrade Flow unsafe as written. The design work in #1920 is good and mostly orthogonal to this; what is missing is a promotion-safety baseline underneath it. #1920 should be reopened as the design track; this RFC is about the precondition.

## What we measured

Two Kubernetes clusters, `kvcacheai/mooncake:0.3.13` (digest `sha256:51d695314243f3a35615784634ac4fd4bfd5f1aca316df997383a0617169b780`), etcd-backed HA, 2 masters, 3 and 7 store clients, `enable_snapshot` tested both ways.

| Upgrade path | Result | Mechanism |
|---|---|---|
| `RollingUpdate` the master Deployment, promote the new standby | **index lost** — 200 keys → 0, and 300 keys → 0 on the second cluster | two distinct causes, below |
| `Recreate` both masters | **index lost** | OpLog replication is standby-to-primary state transfer, not persistence; with `enable_snapshot=false` there is nothing on disk |
| `enable_snapshot=true`, restart, restore | **objects lost** (segments and `Replica::next_id_` return) | filed as #3761 |

Store-client and compute-client rolling upgrades are fine — see "What already works" below. The blocker is entirely the Master.

## The load-bearing problem: neither available readiness signal predicts a safe promotion

We tried the obvious mitigation — create the standby, let it soak under live traffic, then promote — and it failed too. Two attempts, two different causes:

**Attempt 1, a freshly created standby has no baseline.** It reports itself fully synced in **7 microseconds**, having replayed nothing, while the live primary was at sequence 206:

```
17:49:59.719648 oplog_applier.cpp:117 OpLogApplier: recovered from sequence_id=0, expected_sequence_id set to=1
17:49:59.719655 standby_state_machine.cpp:245 Standby state transition: SYNCING -> WATCHING (event: SYNC_COMPLETE)
```

It starts at zero, declares `SYNC_COMPLETE`, and watches *forward* — it never replays history. That is consistent with the documented Standby Bootstrap sequence ("from the snapshot's `last_included_seq`, or from 1 if no snapshot"), so with no snapshot there is nothing to build on. Arguably a design limit rather than a bug — but a promotable one, which is the problem.

**Attempt 2, a soaked standby fails as well, and this one is a bug.** This standby had been up nine minutes *before* the data was written, had `ha_oplog_applied_sequence_id 1920`, `ha_oplog_applied_entries_total 1953`, and had watched every one of the 300 puts:

```
I hot_standby_service.cpp:560] Promoting Standby to Primary. Applied seq_id: 1920, lag: 0 entries
I hot_standby_service.cpp:588] Standby promoted to Primary successfully. All remaining OpLog entries have been synced.
E master_service.cpp:3198] RestoreFromStandbySnapshot: overlapping memory descriptors
E master_service_supervisor.cpp:401] Standby restore failed: INVALID_PARAMS
```

The standby **had** the state. Promotion reported success. Then the restore validator rejected the entire `PromotionContext` on one inconsistency and the index came out empty. Filed separately as #3760. Note this sits *on top of* #3527 and #3354, both already inside `0.3.13` — the error string only exists because of #3354 — so it is not a stale-version problem.

**The consequence for orchestration: `ha_oplog_standby_lag = 0` is not a go signal.** It read 0 throughout *both* failed attempts. It means "not behind on the stream I am watching", and says nothing about whether the restore will be accepted. The only honest number was the standby's own `master_key_count = 0`, and there is no signal at all for "my `PromotionContext` will survive validation". Those two `E` lines are the sole evidence, and **no metric exposes the failure**.

**This is why #1920's Master Upgrade Flow does not hold today.** That flow is:

> Start new-version Standby → wait for `IsReadyForPromotion()` → StepDown old Primary → new Standby wins election → Clients auto re-mount → remove old container

Both of our failed attempts would have satisfied any readiness predicate expressible from the observable state — the fresh standby reaches `WATCHING`/`SYNC_COMPLETE` in 7 µs, and the soaked one reported `lag: 0` with 1953 entries applied. A graceful StepDown API makes the transition *deterministic*; it does not make it *safe*. If `IsReadyForPromotion()` is satisfied by `WATCHING` state (which the observable state machine suggests, though we have not read that predicate), the flow reliably destroys the index. Worth confirming before that design is implemented on top.

## Proposed changes

Ordered by what unblocks what. (1) and (2) are the precondition; (3) and (4) are independently useful and already have issues.

### 1. Validate the promotion context *before* the point of no return, and abort rather than serve empty

Today validation runs after `Promoting Standby to Primary` has already succeeded and the old leader is gone, so a validation failure has no recovery path — it produces a serving master with an empty index and no error surface.

- Run `RestoreFromStandbySnapshot`'s validation as a dry run while the standby is still a standby.
- On failure, **refuse promotion** and leave the existing leader in place. Losing an election attempt is recoverable; losing the index is not.
- Separately, prefer partial restore over total discard: one bad memory descriptor should drop that descriptor, not 300 keys (this is the #3760 ask).

### 2. Expose an honest promotability predicate

An orchestrator needs one boolean it can gate a rollout on, and it must mean "if promoted now, I will come up with the index intact" — not "I am not behind on my stream".

- Add the dry-run validation result from (1) to the standby's admin endpoint, alongside `applied_sequence_id`, the primary's current sequence, and the **key count the standby would restore**.
- `master_key_count` on a standby is already the honest signal and is already exported; making the *validation* outcome observable is the new part.
- Today `Applied seq_id: …, lag: …` appears only in a log line emitted at promotion time — after the decision.

### 3. Bound client reattachment (#3740 / #3743)

Measured at **121.0 s, reproducible to under 1 ms** across two pods and two failovers, while the new master sat `service_ready=true` with `Clients: 0` for 113.6 s of it. Full analysis in #3740. This matters here specifically because a rolling upgrade performs *N* failovers, so the cost multiplies — and #1920's "Current Capabilities" lists clients auto-remounting on `NEED_REMOUNT`, which is exactly what does not happen during that window: Ping is blocked and never returns at all.

### 4. Make version skew loud (covered by #1920 §1, plus one addition)

We reproduced the mixed-version case that #1920 §1 exists to fix: a `0.3.12.post1` client against a `0.3.13` master fails **only on the write path**, with `errc::invalid_rpc_arguments` ("invalid rpc arg") at the client and **nothing whatsoever in the master log**, because the request is rejected before reaching any handler. Reads keep working, so a partial rollout looks healthy.

`struct_pack::compatible` plus the relaxed major-version check in #1920 §1 is the right fix. Until it lands, one cheap addition: have `ServiceReady` log the rejected peer version **master-side**. Right now the operator-visible symptom of a version-skewed rollout is a silent drop in cache hit rate.

## What already works, and should be kept out of scope

Verified by fault injection rather than by reading, so these are load-bearing and fine:

- **Segment drain lifecycle** `OK → DRAINING → DRAINED → UNMOUNTING`. `DRAINING` stays readable and refuses new allocations, exactly as documented. A graceful store-pod delete produces a clean drain; the allocator falls back to healthy segments with no failed writes.
- **Force-killing a store pod** removes its segment from the list cleanly.
- **Compute clients** are stateless and restart without ceremony.
- **A long-lived standby surviving a leader crash keeps everything** — 200/200 objects intact across two `kubectl delete pod ` runs. This is the case that works, and it is the one difference from the rollout case: that standby had been running since before the data was written *and* its context passed validation.

So a store-client rolling upgrade is already achievable today. It is only the Master that has no safe path.

## Environment

`kvcacheai/mooncake:0.3.13` @ `sha256:51d695314243f3a35615784634ac4fd4bfd5f1aca316df997383a0617169b780`, Kubernetes, etcd-backed `LeaderCoordinator`, 2-replica master Deployment, `replica_num=1`, store clients discovering the master via `etcd://`. `MC_RPC_PROTOCOL` / `MC_RPC_CONNECT_TIMEOUT_MS` / `MC_RPC_TIMEOUT_MS` all unset. Both `enable_snapshot=0` and `=1` exercised.

Happy to rerun any of the above on demand — we have this on two clusters and can reproduce the promotion failures reliably. If the dry-run validation in (1) is something maintainers would accept, we can also test a patch against our reproduction.

### Related

- #1920 — the rolling-upgrade RFC this depends on; requesting reopen as the design track
- #3760 — standby promotion discards the whole index on one validation failure
- #3761 — snapshot restore reports success but drops every object
- #3740 — clients take ~121 s to reattach after failover
- #3167 — HA replacement path, noted as under active development in #3561

Contributor guide

Open the contributing guide

Research direction

Start by tracing the standby promotion path around RestoreFromStandbySnapshot and the promotion logs in hot_standby_service.cpp and master_service.cpp. Reproduce the reported promotion failure on Kubernetes with the stated 0.3.13 environment, then determine how a pre-promotion validation result can be exposed. Done means failed validation leaves the current leader in place and the standby reports whether its index can be restored safely.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, kubernetes
Domain
backend-api-design, distributed-systems, observability
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.