Skip to content

feat(gateway): upgrade to wavekv 2.0 delta-state sync - #1031

Open
kvinwang wants to merge 27 commits into
nextfrom
feat/wavekv-v2-dual-stack
Open

feat(gateway): upgrade to wavekv 2.0 delta-state sync#1031
kvinwang wants to merge 27 commits into
nextfrom
feat/wavekv-v2-dual-stack

Conversation

@kvinwang

@kvinwang kvinwang commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Upgrades dstack-gateway to WaveKV 2.0 delta-state replication.

WaveKV 1.0 has only been deployed as a single gateway, not as a multi-node production cluster. This PR therefore does not expose the v1 network protocol or support mixed v1/v2 clusters. Existing single-node installations are upgraded while stopped; their WaveKV 1.0 snapshot and WAL are read in place by WaveKV 2.0.

Changes

Sync protocol

  • Serve sync exchanges at /wavekv/sync/{store}.
  • Serve opportunistic pushes at /wavekv/push/{store} to reduce propagation latency.
  • Keep periodic synchronization as the anti-entropy and ack-authority path.
  • Authenticate both routes with same-app-id mTLS.
  • Bound compressed request/response bodies to 16 MiB and decompressed payloads to 128 MiB.
  • Decode through SyncEnvelope::decode, which validates the schema version and rejects trailing bytes.

There is intentionally no versioned compatibility route, protocol probing, or v1 fallback.

WaveKV 1.0 data migration

The gateway opens the existing WaveKV 1.0 data directory directly. A regression test creates data with the real wavekv = 1.0.0 crate, writes both a snapshot and a trailing WAL entry, then opens the directory through the WaveKV 2.0-backed KvStore and verifies both records are preserved.

This supports the production upgrade model:

  1. Stop the WaveKV 1.0 single-node gateway.
  2. Upgrade the binary.
  3. Start the WaveKV 2.0 gateway with the same data directory.

Admission control

Each store has a key-prefix admission policy enforced inside WaveKV merge processing, covering entries received in both requests and responses. Rejected entries park ack adoption so they cannot be silently skipped.

Observability

WaveKvStatus now reports:

  • State digest, merged-entry count, and rejected-entry count per store.
  • heard_from, digest mismatch count, and consecutive failure count per peer.

Verification

  • cargo test -p dstack-gateway: 120 passed.
  • cargo clippy -p dstack-gateway --all-targets -- -D warnings: passed.
  • cargo fmt --all --check: passed.
  • Explicit WaveKV 1.0 snapshot + WAL migration test: passed.
  • Post-upgrade write, persist, and restart test: passed.
  • Real Rocket HTTPS listener with mandatory client certificates covering sync and push end to end: passed.
  • WaveKvStatus store and peer telemetry mapping test: passed.
  • Gateway sync E2E suite: 6 passed, covering push-before-interval propagation, periodic repair after a missed push, and bootstrap after local store loss with identity preserved.

Dependency

dstack/Cargo.toml temporarily points WaveKV at the WaveKV 2.0 development branch. Repoint it to the released wavekv = "2.0" crate before merging this PR.

Copilot AI lite review requested due to automatic review settings August 8, 2026 10:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR upgrades dstack-gateway’s replicated KV sync layer to wavekv 2.0 (delta-state replication) while remaining compatible with wavekv 1.x peers during rolling upgrades, and adds schema-based admission control plus new sync observability fields exposed via the admin RPC.

Changes:

  • Add dual-stack HTTP sync endpoints (/wavekv/sync v1 + /wavekv/sync2 v2) and an opportunistic push route (/wavekv/push) to reduce propagation latency.
  • Enforce per-store key-shape admission via a new schema policy integrated into wavekv node config.
  • Extend admin/RPC status reporting with per-store digests and per-peer negotiated protocol / mismatch telemetry, and update wavekv dependency to the v2 branch.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
dstack/gateway/src/web_routes/wavekv_sync.rs Adds v2 sync and push HTTP endpoints; refactors gzip handling and introduces envelope decoding.
dstack/gateway/src/web_routes.rs Mounts the new wavekv v2 sync + push routes alongside v1.
dstack/gateway/src/kv/sync_service.rs Extends the sync network interface to use wavekv v2 envelopes and probing for v1/v2 negotiation.
dstack/gateway/src/kv/schema.rs Introduces per-store key admission policy (schema) with tests.
dstack/gateway/src/kv/mod.rs Wires admission policy into wavekv node configs; adds gateway-level wire-compat tests for v1/v2 sync.
dstack/gateway/src/kv/https_client.rs Adds raw-bytes probe POST helper for v2 negotiation and opportunistic push transport.
dstack/gateway/src/admin_service.rs Plumbs new wavekv v2 telemetry (digest, merged/rejected, per-peer protocol/mismatches) into admin RPC responses.
dstack/gateway/rpc/proto/gateway_rpc.proto Extends sync status protos with digest + v2 peer telemetry; deprecates buffered_logs.
dstack/Cargo.toml Switches wavekv dependency to the v2 git branch (with TODO to repoint to crates.io 2.0).
dstack/Cargo.lock Locks wavekv to the v2 git revision and updates transitive deps accordingly.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread dstack/gateway/src/web_routes/wavekv_sync.rs
Comment thread dstack/gateway/src/kv/mod.rs Outdated
Comment thread dstack/gateway/src/web_routes/wavekv_sync.rs Outdated
Pick up the wavekv fix for the opportunistic push envelope, which was built
without a `sender_uuid` and so failed `check_uuid` on every push — this gateway
implements `query_uuid`, so the push channel never worked here. Writes still
converged over the periodic round, but each one waited a full sync interval
instead of the coalesce window and the receiver logged an error per push
blaming node-id reuse.

That fix also widens `link_status` to report every known peer rather than only
those in the link cache. A peer whose rounds all fail was previously absent
from `WaveKvStatus` entirely: a 5xx deliberately does not demote a peer to
"v1", so nothing about it moved. Report the new `consecutive_failures` streak
so that stall is visible.

Document the one direction in which the store schema is not forward
compatible: values may gain fields freely, but a new *key* is rejected by nodes
that predate it, and a rejection parks ack adoption for the whole round (rule
R1). The pair then re-exchanges the same batch indefinitely with no error. New
keys therefore ship in two releases — widen the schema everywhere first, write
the key second.

Also silence a `manual_repeat_n` lint in the pp tests, unrelated but newly
raised by the toolchain and enough to fail `clippy -D warnings`.
The HTTP layer was the one part of the sync path with no coverage. It was
skipped on the grounds that constructing a `WaveKvSyncService` needs real TLS
material; that was wrong. `rcgen` is already a dependency and already used by
the cert_store tests, and `verify_gateway_peer` short-circuits under
`insecure_skip_attestation`, so a self-signed CA plus a leaf written to a
TempDir is enough to build a serving gateway.

What this pins that nothing else did:

- 503, not 404, when sync is disabled. 404 is the negotiation signal, so a
  sync-disabled node answering 404 would be cached as "v1" by every peer for a
  whole reprobe window — and sync is off, so nothing would correct it.
- 404 for an unknown store, which is the same signal used deliberately.
- An unstamped push is refused at the route and writes nothing. This is the
  server-side view of the envelope-identity bug; the sender-side view lives in
  the wavekv push test.
- A well-formed push reaches the store, a v2 round trip returns a decodable
  envelope, and node id 0 is refused.

Also stop reporting a 404 on the push route as a delivered push.
`post_bytes_probe` maps 404/405 to `Ok(None)` so the v2 probe can read it as
"not upgraded yet", but `push_to` discarded the `Option`. A mistyped push URL
was therefore indistinguishable from success — the same shape of silent failure
that let the unstamped-envelope bug survive, since pushes are best-effort and
only debug-logged.
Takes the wavekv fix that verifies the responder's uuid on a v2 response. The
field was already on the wire and populated by the responder; only the initiator
never read it, so node-id-reuse detection ran in one direction.
The responder-side identity check shipped in the previous bump wedged any peer
that regenerated its uuid — an ordinary CVM rebuild, since the uuid is derived
from the data directory while the node id comes from config.
The sync wire is gzipped and the 16 MiB cap on a request body caps the
*compressed* size, which bounds nothing on its own — gzip expands by three
orders of magnitude on attacker-chosen input, so that cap admits a payload that
expands into the gigabytes. Every gateway in a cluster shares one app_id, so
mTLS proves only that the sender is some gateway of this deployment; it is the
same trust level the key schema already treats as insufficient.

All four decompression points are now bounded through one helper: both server
routes and both client response paths. The client also read peer responses with
`Body::collect`, which has no limit at all, so the memory was already spent
before any decoding bound could apply; response bodies now go through `Limited`
with the same 16 MiB the routes accept on a request.

The decompressed ceiling is 128 MiB, far above any legitimate payload: a v2
delta is capped by `max_delta_bytes` at 4 MiB, and the v1 shim answers with the
whole live state, which is bounded by the gateway's own key set rather than by
anything a peer controls.

Tested at the limit as well as past it — a fixture landing exactly on the
ceiling must still decode, or the bound could tighten by a byte with only the
bomb test still passing.
A local write allocates a sequence number. After a data-directory loss the node
keeps its id but has no record of which numbers it already spent — only its
peers do — so `bootstrap` rebuilds the counter from their coverage. Anything
written before that reuses numbers the peers already treat as seen, and peers
filter those writes out of every delta with no error on either side.

The three records written at startup were exactly the ones that must not be
dropped: `node/info` carries the fresh uuid peers check us against, and
`__peer_addr` carries the address they route to. A rebuilt gateway therefore
wedged in both directions and stayed wedged until per-peer digest repair
happened to fire.

They could not simply be moved, because `HttpSyncNetwork::new` read this node's
uuid back out of the store, making the `node/info` write a prerequisite of
building the sync service at all. That read is the actual defect: our own uuid
is local configuration, not replicated state, and routing it through the store
created the ordering constraint that forced the bug. It is now passed in, and
all three writes happen after the bootstrap.

Also bound the response body in `post_json`. Every other response is read
through `read_body_bounded`; this one collected without a limit, so a peer could
stream until memory ran out. It is the bootnode GetPeers path, and the threat
model does not assume a bootnode is honest.
Picks up: WAL truncation of a damaged tail before appending (writes after a
torn-tail recovery were silently lost on the next restart), the reset_acks hint
the divergence repair always needed (repair reached only entries the peer itself
authored), sequence-number recovery that survives an own entry losing LWW,
cross-page R1 enforcement, and requests no longer disclosing our state digest.

The wire test framed a *request* to check the digest survives transport. Requests
no longer carry one, so it frames a response — the direction the digest actually
travels — and asserts the request has none.
…ity fixes

Retiring a peer no longer discards our coverage of the entries it authored,
which is what let the rest of the cluster compute a GC watermark for that
origin; membership ops are now WAL-durable, so a peer lost to a crash can no
longer widen the watermark and resurrect that peer's deletes.
Mutation testing found `verify_gateway_peer` replaceable with `Ok(())` without
turning the suite red. The sync routes are the cluster's write surface — anything
reaching them inserts entries that replicate to every gateway — and that function
is the only thing in front of them.

The cause was in the fixture: every route test sets `insecure_skip_attestation`,
which is the function's first statement, so no test had ever executed a line of
the gate. The comment claimed the flag "stands in for the mTLS peer check". It
does not stand in for it; it removes it.

Two gaps, so two changes.

`enforcing_gateway` runs with the bypass off. Rocket's local client speaks no
TLS and so presents no certificate, which is exactly the case that must be
refused, and all three routes are asserted to answer 401.

The app-id comparison needed a certificate, and `rocket::mtls::Certificate` has
no public constructor — it exists only as the output of a real handshake. But
the adapter over it only ever used `cert.extensions()`, which is public and
whose element type comes straight out of `X509Certificate`. `RocketCert` now
holds the extension list, so a test can build one from a certificate minted in
process, and the authorization rule is split out from the Rocket plumbing it was
tangled with.

None of this needs a TEE or a simulator: the check reads two X.509 extensions
and compares bytes. `CertRequest` adds `PHALA_RATLS_APP_ID` unconditionally, and
the gateway's own app id is already a constructor parameter.

Four cases now pinned: matching id accepted, foreign id forbidden, certificate
without an app id refused, and a gateway with no app id of its own authorizing
nobody. Each was verified to die under the mutation it targets.
The v1 route had no round-trip test. Mutation testing could delete either store
arm, invert the node-id-zero guard, or replace the whole response body with three
bytes, and the suite stayed green — every route test targeted v2, because v1 is
the compatibility path and attention went to the new one.

Deleting the `"persistent"` arm is the sharpest of these. It falls through to
`_ => 404`, and a 404 on a sync route is exactly the signal a v2 peer reads as
"this node has no such route" — so a broken store dispatch would not surface as
an error, it would surface as a successful protocol downgrade, cluster-wide and
silently, for a whole reprobe window. The suite already documents that reasoning
for the sync-disabled 503 case; the v1 route just had nothing enforcing it.

Three tests: a round trip that asserts the response decodes and carries the state
this node holds, the same for the ephemeral store, and a node-id-zero rejection
matching the push and v2 routes.
…d the key schema

Three gaps mutation testing found, none of which needed any infrastructure — all
three are pure functions over bytes.

`AppIdValidator::validate` had no tests at all. It runs during the TLS handshake,
so a validator that always returns `Ok(())` means this gateway completes a
mutually-authenticated connection to any peer holding any certificate our CA
signed, and then sends it our state. It is the client-side mirror of the route
check covered in 8d04ff2, and it was equally undefended: replacing the body with
`Ok(())` or inverting the comparison left the suite green.

The decompression-limit test asserted a payload of exactly
`MAX_DECOMPRESSED_SYNC_BYTES` is accepted — building that payload from the same
constant. It therefore held for whatever the constant said, and shrinking 128 MiB
to a few kilobytes kept it green while rejecting every real delta. It pinned `>`
against `>=` and nothing else. The limits are now checked against what the
protocol actually produces: room for wavekv's 4 MiB delta cap, and a compressed
ceiling equal to what the routes accept on a request.

The key namespace had no tests either. Every builder and parser survived
mutation: `handshake_prefix` could return `""`, `parse_inst_key` could return
`Some("xyzzy")`. These strings are how a gateway finds its own state after an
upgrade, so changing one orphans every existing record — still replicated, still
in the digest, unreachable by any reader. Four properties are now pinned: a
prefix matches the keys it iterates, a prefix does not capture a neighbour
(`inst-a` must not swallow `inst-ab`), builders and parsers round-trip, and a
parser refuses a key from another namespace.
A node rebuilt from an empty data directory no longer adopts a requester's ack
map while it is bootstrapping — the window in which its own coverage is unknown
to it, and in which adopting would have it claim coverage of state it does not
hold.
`post_bytes_probe`'s mapping of 404/405 to `Ok(None)` *is* the v1/v2 negotiation:
a gateway that has not been upgraded has no `/wavekv/sync2` route, and that
status is the only signal its peers get. Every mutation of the condition
survived — `||` to `&&`, either `==` to `!=`, the `!` on `is_success` — because
nothing exercised the function at all. It cannot be reached without a peer that
speaks TLS, since the client is built `https_only()`, and that was enough
friction for the whole file to sit at zero.

A listener on 127.0.0.1 with a certificate minted in process is enough. No
container, no simulator: nothing on this path verifies a quote.

Four cases: both "no such route" statuses read as not-upgraded; a 5xx or 4xx
stays an error, because reading one as not-upgraded would demote a healthy v2
peer to the v1 path for a whole reprobe window; a 200 is decompressed and
returned; and an oversized body is refused.

The last one initially passed with the bound removed entirely. Its payload was
not valid gzip, so `gunzip_bounded` rejected it whatever the ceiling said, and
the assertion measured nothing. It now sends stored-mode gzip — valid, and large
enough to clear the compressed ceiling while decompressing well inside the
decompressed one — so only the bound under test can reject it.
Three survivors were left on the TLS client after the negotiation tests, and all
three sit on paths this cluster depends on.

`post_compressed_msg` is the v1 sync path — how a v2 gateway talks to one that
has not been upgraded. Its status check was as untested as the negotiation's, so
a v1 peer answering 500 could have been decoded as a successful round. `post_json`
is the bootnode GetPeers path, where the threat model does not assume the peer is
honest, and a failure status must not be parsed as a peer list.

The third needed the custom-verifier path. `AppIdValidator` runs inside
`CustomCertVerifier`, which rustls only reaches once standard chain verification
passes, so unit-testing the validator alone leaves the wiring between them
untested — and the wiring is what decides whether a peer from another app can
open a connection at all. It now serves a certificate carrying a foreign app id
and asserts the handshake fails before any application bytes move, with the
matching id as the control. Deleting the validator call turns it red.
@kvinwang
kvinwang force-pushed the feat/wavekv-v2-dual-stack branch from 9a6e217 to e28384f Compare August 11, 2026 04:26
kvinwang added a commit that referenced this pull request Aug 11, 2026
fix(gateway): harden the WaveKV sync path (extracted from #1031, no protocol change)
Comment thread dstack/gateway/src/kv/schema.rs Dismissed
@kvinwang kvinwang changed the title feat(gateway): upgrade to wavekv 2.0 delta-state sync with v1 dual-stack feat(gateway): upgrade to wavekv 2.0 delta-state sync Aug 17, 2026
@kvinwang
kvinwang requested a lite review from Copilot August 17, 2026 13:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants