diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index 1c0794fa3..119c2c35d 100644 --- a/docs/admin_ui_key_visualizer_design.md +++ b/docs/admin_ui_key_visualizer_design.md @@ -38,11 +38,12 @@ This document proposes a built-in admin Web UI, shipped as a separate binary `cm ```mermaid flowchart LR - Browser["Browser (Svelte SPA, embedded)"] + Browser["Browser (embedded KeyViz SPA)"] + Standalone["cmd/elastickv-admin (overview path)"] - subgraph AdminHost["Operator machine or sidecar"] - Admin["cmd/elastickv-admin :8080"] - end + Node1HTTP["Node A embedded admin HTTP"] + Node2HTTP["Node B embedded admin HTTP"] + Node3HTTP["Node C embedded admin HTTP"] subgraph Cluster["Elastickv Cluster"] Node1["Node A"] @@ -50,10 +51,13 @@ flowchart LR Node3["Node C"] end - Browser -- "HTTP/JSON + WebSocket" --> Admin - Admin -- "gRPC: Distribution, RaftAdmin, Admin.KeyViz" --> Node1 - Admin -- "gRPC" --> Node2 - Admin -- "gRPC" --> Node3 + Browser -- "HTTP/JSON polling" --> Node1HTTP + Node1HTTP -- "admin HTTP peer fan-out" --> Node2HTTP + Node1HTTP -- "admin HTTP peer fan-out" --> Node3HTTP + + Standalone -- "gRPC overview path" --> Node1 + Standalone -- "gRPC overview path" --> Node2 + Standalone -- "gRPC overview path" --> Node3 subgraph NodeInternal["Inside each Node"] Sampler["keyviz.Sampler"] @@ -69,20 +73,36 @@ flowchart LR AdminSvc --> Raft ``` -The admin binary holds no authoritative state. All data is fetched on demand from nodes via a new `Admin` gRPC service. The sampler's ring buffer lives inside each node's process, rebuildable after restart once Phase 3 persistence is enabled (see §5.6). - -### 3.1 Why a separate binary - -- Release cadence for the UI is decoupled from the data plane. -- The admin binary can be placed on an operator workstation or a sidecar pod, so a compromised UI does not imply a compromised data node. -- Node binaries remain free of the Prometheus client (goal §2.1-6) and of any SPA assets. -- `cmd/elastickv-admin --nodes=host:50051 --nodeTokenFile=/etc/elastickv/admin.token` is the full invocation; no multi-file config bundle is required for the default use case. +The original standalone admin binary remains a stateless gRPC overview +client. The implemented KeyViz fan-out path is embedded in each +`elastickv` server: the browser talks to one node's admin HTTP handler, +that handler computes its local matrix, then it issues admin HTTP peer +requests to the static `--keyvizFanoutNodes` list and merges the results. +The sampler's ring buffer lives inside each node's process, rebuildable +after restart once Phase 3 persistence is enabled (see §5.6). + +### 3.1 Standalone and embedded admin surfaces + +- The original `cmd/elastickv-admin` binary is retained as a + gRPC-based overview client for Phase 0 diagnostics. It can run on an + operator workstation or sidecar pod when operators want a client-only + view of node metadata. +- The shipped KeyViz SPA and Phase 2-C fan-out path are embedded in each + `elastickv` server process. Server builds therefore carry the admin + HTTP handlers and SPA assets for this path; the standalone overview + binary is not the KeyViz delivery vehicle. +- The sampler path still avoids a Prometheus client dependency in the + data-plane hot path (goal §2.1-6). +- For the standalone overview path, + `cmd/elastickv-admin --nodes=host:50051 --nodeTokenFile=/etc/elastickv/admin.token` + remains the full invocation; KeyViz Phase 2-C fan-out uses the + embedded HTTP handler described in §9.1 instead. ## 4. API Surface Two layers: -**Layer A — gRPC, node → admin binary.** A new `Admin` service on each node, registered on the same gRPC port as `RawKV` (`--address`, default `:50051`). All methods are read-only in Phases 0–3 and require `authorization: Bearer ` metadata. Nodes load the token from `--adminTokenFile`; the admin binary sends it from `--nodeTokenFile`. An explicit `--adminInsecureNoAuth` flag exists only for local development and logs a warning at startup. +**Layer A — gRPC, node → standalone admin binary.** A new `Admin` service on each node, registered on the same gRPC port as `RawKV` (`--address`, default `:50051`). All methods are read-only in Phases 0–3 and require `authorization: Bearer ` metadata. Nodes load the token from `--adminTokenFile`; the standalone admin binary sends it from `--nodeTokenFile`. An explicit `--adminInsecureNoAuth` flag exists only for local development and logs a warning at startup. This layer remains the Phase 0 overview path; the implemented KeyViz SPA and Phase 2-C fan-out use the embedded admin HTTP surface below. | RPC | Purpose | |---|---| @@ -90,22 +110,19 @@ Two layers: | `ListRoutes` | Existing `Distribution.ListRoutes` (reused, not duplicated) | | `GetRaftGroups` | Per-group state (leader, term, commit/applied, last contact) | | `GetAdapterSummary` | Per-adapter QPS and latency quantiles from the in-process aggregator | -| `GetKeyVizMatrix` | Heatmap matrix for **this node's locally observed samples**: leader writes plus reads served locally, including follower-local reads (see §5.1). The admin binary fans out and merges. | -| `GetRouteDetail` | Time series for one route or virtual bucket (drill-down). The admin binary fans out because reads may be observed by followers. | +| `GetKeyVizMatrix` | Original gRPC matrix surface for **this node's locally observed samples**. The shipped Phase 2-C SPA does not call this path; it uses `/admin/api/v1/keyviz/matrix` on an embedded server and lets that handler perform HTTP fan-out. | +| `GetRouteDetail` | Original drill-down surface for one route or virtual bucket. Embedded KeyViz drill-down endpoints live under `/admin/api/v1/keyviz/*`. | | `StreamEvents` | Server-stream of route-state transitions and fresh matrix columns | -**Layer B — HTTP/JSON, browser → admin binary.** Thin pass-through wrappers over the gRPC calls, plus static asset serving. +**Layer B — HTTP/JSON, browser → embedded admin handler.** Implemented server routes live under `/admin/api/v1/*` and are mounted by each `elastickv` process when `--adminEnabled` is set. `cmd/elastickv-admin` still has a small `/api/cluster/overview` wrapper for the standalone overview workflow, but it is not the KeyViz delivery vehicle. | Method | Path | Purpose | |---|---|---| -| GET | `/` (and `/assets/*`) | Embedded SPA | -| GET | `/api/cluster/overview` | Wraps `GetClusterOverview` | -| GET | `/api/routes` | Wraps `ListRoutes` + derived size/leader | -| GET | `/api/raft/groups` | Wraps `GetRaftGroups` | -| GET | `/api/adapters/summary` | Wraps `GetAdapterSummary` | -| GET | `/api/keyviz/matrix` | Wraps `GetKeyVizMatrix` | -| GET | `/api/keyviz/buckets/{bucketID}` | Wraps `GetRouteDetail` for a real route bucket or coarsened virtual bucket | -| WS | `/api/stream` | Multiplexes `StreamEvents` from all targeted nodes | +| GET | `/admin/` (and `/admin/assets/*`) | Embedded SPA served by the node process | +| POST | `/admin/api/v1/auth/login` | Browser login; mints `admin_session` and `admin_csrf` cookies | +| GET | `/admin/api/v1/cluster` | Embedded cluster overview | +| GET | `/admin/api/v1/keyviz/matrix` | Local KeyViz matrix plus optional Phase 2-C HTTP peer fan-out | +| GET | `/admin/api/v1/keyviz/hotkeys` | Hot-key drill-down when `--keyvizHotKeysEnabled` is set | HTTP errors use a minimal `{code, message}` envelope. No caching headers on read endpoints. @@ -140,7 +157,22 @@ sampler.Observe(routeID, key, op, valueLen, label) `sampler` is an interface; the default implementation is nil-safe (a nil sampler compiles to one branch and no allocation). The hook runs *before* Raft proposal so it measures offered load, not applied load. -Writes are sampled exactly once by the current Raft leader before proposal. Reads are sampled by the node that actually serves the read: leader reads are marked `leaderRead`, and lease/follower-local reads are marked `followerRead`. Requests forwarded between nodes carry an internal "already sampled" marker so a logical operation is not counted twice. Because read load can be spread across followers, a cluster-wide heatmap requires the admin binary to fan out and merge across nodes (§9.1) — pointing at a single node would produce a partial view. +Writes are sampled at the ingress `ShardedCoordinator` before Raft +forwarding or proposal, so write cells are offered-load observations +from the node that accepted the request rather than exact leader-only +applied-write counters. In follower-ingress or multi-ingress +deployments, two nodes can legitimately report same-term write samples +for the same bucket. The fan-out merge dedupes by +`(bucketID, raftGroupID, leaderTerm, column)` when identity is present +and treats same-identity disagreements as ambiguity, not necessarily as +a Raft safety anomaly. Reads are sampled by the node that actually +serves the read: leader reads are marked `leaderRead`, and +lease/follower-local reads are marked `followerRead`. Requests forwarded +between nodes carry an internal "already sampled" marker so a logical +operation is not counted twice. Because read load can be spread across +followers, a cluster-wide heatmap requires embedded admin HTTP fan-out +and server-side merge across nodes (§9.1) — pointing at a single node +would produce a partial view. **Leadership loss.** Each sample carries the `(raftGroupID, leaderTerm)` under which it was recorded. When the node's lease-loss callback fires for a group, the sampler stamps all `leaderWrite` samples for that group in the current and previous step window with `staleLeader=true` rather than deleting them — keeping them visible on the heatmap helps operators diagnose rapid leadership churn, and they remain authoritative for the window in which this node was in fact the leader. The admin fan-out (§9.1) merges writes by `(bucketID, raftGroupID, leaderTerm, windowStart)`, so the stale samples from an old leader and the fresh samples from a new leader never double-count: distinct terms are summed (each term's leader only saw its own term's writes), and within a single term the one leader's samples are authoritative. If fan-out receives `staleLeader=true` samples that conflict with a concurrent newer-term sample for the same window, the cell is flagged `conflict=true` and rendered hatched. @@ -276,20 +308,36 @@ Phases 0–2 require no Raft or FSM changes. Data-plane protocol adapters only r ## 9. Deployment and Operation -- The admin binary is not intended to be exposed on the public network in its initial form. Default bind is `127.0.0.1:8080`; browser login and RBAC are deferred, but node-side `Admin` gRPC calls require the shared read-only token from §4. -- Typical operator workflow: `ssh -L 8080:localhost:8080 operator@host` then `elastickv-admin --nodes=host1:50051,host2:50051,host3:50051 --nodeTokenFile=/etc/elastickv/admin.token`, or run the binary on a laptop and point it at any reachable subset of nodes. -- The admin binary is stateless; it can be killed and restarted without coordination. +- The original standalone admin binary remains a local control-plane + overview tool, but it is not the shipped KeyViz fan-out surface. The + implemented KeyViz SPA and Phase 2-C fan-out are served by each + `elastickv` server's embedded admin HTTP handler. +- Typical Phase 2-C operator workflow: enable the embedded admin handler + on every node with shared admin login prerequisites, set + `--keyvizEnabled`, and configure the same static + `--keyvizFanoutNodes` HTTP endpoint list on every node. Operators may + still use SSH port forwarding for private access, but the browser logs + into a server admin endpoint rather than into + `cmd/elastickv-admin --nodes`. +- The embedded fan-out path is stateless outside the node-local sampler + buffers; restarting one node degrades that node's recent local samples + until traffic repopulates them, while other nodes continue to answer + peer requests. - CI produces release artifacts for `linux/amd64`, `linux/arm64`, `darwin/arm64`, and `windows/amd64`. ### 9.1 Cluster-wide fan-out -Because writes are recorded by Raft leaders and follower-local reads are recorded by the followers that serve them (§5.1), pointing the admin binary at a single node produces a **partial heatmap**. To give operators a complete view by default, the admin binary runs in **fan-out mode**: +Because write cells are ingress-observed offered-load samples and +follower-local reads are recorded by the followers that serve them +(§5.1), a single node's local admin view produces a **partial heatmap**. +Phase 2-C ships a narrower embedded fan-out path inside each server +process: -- `--nodes` accepts a comma-separated list of seed addresses. The admin binary calls `GetClusterOverview` on any reachable seed to discover the current full membership (node → gRPC endpoint, plus per-group leader identity). Membership is cached for `--nodesRefreshInterval` (**default 15 s**) so a stampede of concurrent browser requests hits at most one `GetClusterOverview` per interval per seed, while scale-out events are still reflected within seconds. The cache is refreshed lazily on the first request after expiry and invalidated immediately on any per-node `Unavailable` error, so removed or replaced nodes are dropped on the next request instead of waiting for the next tick. -- For each query (`GetKeyVizMatrix`, `GetRouteDetail`, `GetAdapterSummary`), the admin binary issues parallel gRPC calls to every known node and merges results server-side before sending one combined JSON payload to the browser. -- Merging rule for the heatmap: rows are grouped by `bucketID`/`lineageID` and time step. Read samples from multiple nodes are **summed**, because they represent distinct locally served reads. For write samples the authoritative identity is `(raftGroupID, leaderTerm)` — by Raft invariants at most one leader exists per term per group — so the admin binary collapses write samples to **one value per `(bucketID, raftGroupID, leaderTerm, windowStart)`** key. If the same logical key arrives from more than one node (e.g., an ex-leader that has not yet expired its local cache plus a correctly-responding new leader in the same term), the entries are expected to be identical and the merger keeps one; if they differ, the cell is surfaced with `conflict=true` (not silently dropped). Across distinct `leaderTerm` values for the same group and window, values are summed because each term's leader only observed its own term's writes. The admin binary never uses "later timestamp wins" to overwrite a previous leader's complete window with a new leader's partial window. -- Degraded mode: if any node is unreachable, the admin binary returns a partial result with a per-node `{node, ok, error}` status array so the UI can surface "3 of 4 nodes responded" instead of silently hiding ranges. The heatmap hatches rows or time windows whose expected source node failed. -- A single-node mode — pass one address to `--nodes` and the admin binary will fan out to just that node's view. A future `--no-fanout` flag that also suppresses the background membership-discovery RPC is deferred; for now the operator can simulate it by pointing at a single seed and accepting the one-node partial view. +- `--keyvizFanoutNodes` accepts a comma-separated static list of peer admin HTTP endpoints. Empty disables fan-out. The local node is added in-process, and the configured peers are queried over the existing admin HTTP surface with the `X-Admin-Fanout-Peer` recursion guard. Phase 2-C does not use `cmd/elastickv-admin --nodes`, `GetClusterOverview` membership discovery, or `--nodesRefreshInterval`; dynamic discovery remains a future extension. +- For each KeyViz matrix request, the embedded admin handler computes the local matrix, issues parallel HTTP calls to the configured peers, and merges results server-side before sending one combined JSON payload to the browser. +- Merging rule for the heatmap: reads are **summed** across nodes because they represent distinct locally served reads. Writes use the canonical per-cell identity `(bucketID, raftGroupID, leaderTerm, column)`: duplicates within the same identity are collapsed, same-identity value disagreements set `conflict=true` / `conflicts[column]=true`, and distinct leader terms are summed because each term's leader observed its own writes. The merger never uses "later timestamp wins" to overwrite a previous leader's complete window with a new leader's partial window. +- Degraded mode: if any configured peer is unreachable, the embedded handler returns a partial result with a per-node `{node, ok, error}` status array so the UI can surface "3 of 4 nodes responded" instead of silently hiding ranges. `error` is omitted for successful nodes. +- Single-node mode is the empty `--keyvizFanoutNodes` default: the response is the local node's view, and no peer requests are issued. ## 10. Performance Considerations @@ -298,7 +346,10 @@ Because writes are recorded by Raft leaders and follower-local reads are recorde - The flush goroutine performs in-place `atomic.SwapUint64` per tracked counter; there is no write lock covering `Observe` calls and no retired pointers for late writers to hit. Splits and merges publish a copied immutable route table with child counters before publishing the new `RouteID` (§5.4), so the callback does not race with the hot path. - API endpoints cap `to − from` at 7 days and `rows` at 1024 to bound server work. - `LiveSummary` adds a second atomic increment alongside each existing Prometheus `Inc()`, plus one atomic increment on a fixed-bucket histogram counter. Cost is on the order of a nanosecond and well below the noise floor in §5.2. -- Fan-out cost (§9.1) is N parallel gRPC calls; each node serves only its locally observed samples, so the response size is distributed and the aggregate wall-clock is bounded by the slowest node, not the sum. +- Fan-out cost (§9.1) is N parallel admin HTTP peer requests; each node + serves only its locally observed samples, so the response size is + distributed and the aggregate wall-clock is bounded by the slowest + node, not the sum. ## 11. Testing @@ -306,7 +357,7 @@ Because writes are recorded by Raft leaders and follower-local reads are recorde 2. Route-budget test: generate more than `--keyvizMaxTrackedRoutes` routes and assert that coarsening preserves total observed traffic, keeps hot routes un-merged, and returns `aggregate`, `bucketID`, `routeCount`, and constituent route metadata correctly. 3. Integration test in `kv/` that drives synthetic traffic through the coordinator and asserts the matrix reflects the skew. 4. gRPC handler tests with a fake engine and fake Raft status reader. -5. Fan-out test: admin binary against a 3-node fake cluster, including follower-local reads, one unreachable node, and a leadership transfer in the middle of a step window; the merged response must sum non-duplicate samples, preserve the partial-status array, and flag ambiguous overlap. +5. Fan-out test: embedded admin HTTP handler against a 3-node fake cluster, including follower-local reads, one unreachable node, and a leadership transfer in the middle of a step window; the merged response must sum non-duplicate samples, preserve the partial-status array, and flag ambiguous overlap. 6. Persistence test: write compacted columns to per-range groups, perform split and merge transitions, restart a node, take a leadership transfer, run KeyViz GC, and verify the lineage reader reconstructs complete history across groups without relying on stable `RouteID`s`. 7. Namespace isolation test: user `ScanAt`, `ReverseScanAt`, and `maxLatestCommitTS` must ignore `!admin|keyviz|*` records, and user-plane `Put` / `Delete` / transactional writes to any `!admin|*` key must be rejected with `InvalidArgument` by every adapter (gRPC `RawKV`/`TransactionalKV`, Redis, DynamoDB, S3). 8. Auth test: `Admin` gRPC methods reject missing or wrong tokens and accept the configured read-only token. @@ -321,11 +372,11 @@ Because writes are recorded by Raft leaders and follower-local reads are recorde | 1 | Overview, Routes, Raft Groups, Adapters pages. `LiveSummary` added. No sampler. | All read-only pages match `grpcurl` ground truth. | | 2-A | Key Visualizer MVP server side: in-memory sampler with adaptive sub-sampling, leader writes, leader/follower reads, static matrix API with virtual-bucket metadata. | Benchmark gate green; ±5% / 95%-CI accuracy SLO holds under synthetic bursts; matrix endpoint returns the local node's view. | | 2-B | KeyViz SPA integration into `web/admin/`: heatmap page, series picker, row budget, manual + auto refresh. See `docs/design/2026_04_27_implemented_keyviz_spa_integration.md`. | Heatmap shows synthetic hotspot within ~5 s of `make client` driving traffic against `make run`; type check (`tsc -b --noEmit`) clean. | -| 2-C | Cluster fan-out: admin RPC that aggregates each node's local sampler view so the SPA shows a cluster-wide heatmap rather than the local node's slice. | Fan-out returns complete view with 1 node down; SPA renders aggregate within the §10 budget. | +| 2-C | Cluster fan-out: embedded admin HTTP fan-out that aggregates each node's local sampler view so the SPA shows a cluster-wide heatmap rather than the local node's slice. | Fan-out returns a partial cluster view with per-node failure status when a peer is down; SPA renders the aggregate within the §10 budget. | | 3 | Drill-down, split/merge continuity, namespace-isolated persistence of compacted columns distributed **per owning Raft group**, lineage recovery, and retention GC. | Heatmap remains continuous across a live `SplitRange`; restart preserves last 7 days; expired data and stale lineage records are collected; no single Raft group sees more than its share of KeyViz writes. | | 4 (deferred) | Mutating admin operations (`SplitRange` from UI), browser login, RBAC, and identity-provider integration. Out of scope for this design; a follow-up design will cover it. | — | -Phases 0–2 (A/B/C) together are the minimum operationally useful product; Phase 3 is the "ship-quality" target. As of 2026-07-07, Phase 2-A is shipped (PRs #639/#645/#646/#647/#651/#660/#661/#672), Phase 2-B is shipped, and Phase 2-C is tracked by the implemented cluster fan-out design. Bytes series, originally listed under Phase 3, was rolled forward into 2-A and is already on the wire. +Phases 0–2 (A/B/C) together are the minimum operationally useful product; Phase 3 is the "ship-quality" target. As of the Phase 2-C implementation update, Phase 2-A is shipped (PRs #639/#645/#646/#647/#651/#660/#661/#672), Phase 2-B is shipped, and Phase 2-C is shipped via the cluster fan-out design. Bytes series, originally listed under Phase 3, was rolled forward into 2-A and is already on the wire. ## 13. Open Questions diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md new file mode 100644 index 000000000..02614cb65 --- /dev/null +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -0,0 +1,493 @@ +--- +status: implemented +phase: 2-C +parent_design: docs/admin_ui_key_visualizer_design.md +date: 2026-04-27 +--- + +# KeyViz Cluster Fan-out (Phase 2-C) + +## 1. Background + +Phase 2-A (server-side sampler) and Phase 2-B (SPA heatmap) ship a +heatmap that shows **only the local node's view**. Per the parent +design §5.1 reads are recorded by the follower that serves them and +writes are recorded by the leader; pointing the SPA at a single node +therefore shows a partial picture — operators looking at a follower +see no writes, operators looking at a leader see only that leader's +writes. + +Parent design §9.1 defines the desired end state: the admin layer +fans out to every node, merges responses with rules that respect +Raft leadership, and renders one combined heatmap with degraded-node +status surfaced inline. + +**Rollout notes** (a round-1 review finding on PR #685 asked for explicit +rolling-upgrade and zero-downtime cutover plans). This change is +**off by default** (`--keyvizFanoutNodes` empty) and lives entirely +on the admin path — no data-plane impact, no schema migration, no +state stored on disk by this feature. Mixed-version clusters mid- +rollout are correct by construction: + +- Wire format additions (§5) are forwards-compatible: an old node + that does not know about `Fanout` / `conflict` simply omits them + and the new aggregator treats the missing values as the merge-rule + identity. An old SPA against a new server ignores the extra + fields, but it still renders the merged cluster `rows` returned by + the server. The compatibility risk is presentation-only: old + clients do not show the degraded-node or conflict affordances. +- Operators flip `--keyvizFanoutNodes` per-node during the rollout. + Until every node has the new binary AND the flag set, some peer + HTTP calls will 404 — that surfaces as `ok=false` in the + per-node status array, not as a failed user request. +- No dual-write proxy / blue-green required because there is no + state to migrate; the worst-case partial deploy is "node X + reports `ok=false`", not data loss. + +This design scopes the implemented **Phase 2-C** cluster view: +cluster-wide heatmap, degraded-node banner, and the per-cell Raft +identity needed for the canonical §9.1 write merge. + +## 2. Scope + +### 2.1 In scope + +- A static node list configured via `--keyvizFanoutNodes` (comma- + separated admin HTTP endpoints). +- An aggregator inside `internal/admin` that issues parallel GETs to + every configured node's `/admin/api/v1/keyviz/matrix`, merges the + responses, and returns one combined `KeyVizMatrix` plus a per-node + status array. +- Two merge rules, justified in §4: + - **Reads**: sum across nodes (each node serves distinct local + follower reads). + - **Writes**: canonical per-cell fan-out merge keyed by + `(bucketID, raftGroupID, leaderTerm, column)`: max within one + `(group, term)` identity, sum across distinct leader terms for + the same group/window, and surface `conflicts[j]=true` when + two sources disagree inside the same `(group, term, column)` or + when the unknown-identity fallback sees distinct non-zero values. + The row-level `conflict` field remains as the OR of + `conflicts[]` for older SPAs. If any source lacks + `raftGroupID` or `leaderTerm`, that cell falls back to the + legacy max-merge so mixed-version clusters never over-count. +- Degraded-mode response: when N nodes respond and M < N succeed, + the response carries `{node, ok, error}` per node and the SPA + shows a banner. +- Behaviour preserved when `--keyvizFanoutNodes` is unset: the + endpoint serves the local view exactly as it does today. + +### 2.2 Explicitly NOT in scope + +- **Membership discovery via `GetClusterOverview`** and a membership + cache. Static `--keyvizFanoutNodes` is the implemented Phase 2-C + contract; discovery is a later operator-ergonomics feature, not a + merge-semantics prerequisite. +- **Standalone `cmd/elastickv-admin` binary** (parent §3.1). + Deferred indefinitely — we have not seen a use case the embedded + in-node admin does not cover, and this design holds the + embedded path as the go-forward. +- **Response-payload caching across browser polls.** Each request + fans out fresh. Coalescing concurrent requests is a tomorrow + problem if it shows up in benchmarks. + +## 3. Configuration + +```sh +elastickv \ + --address 10.0.0.1:50051 \ + --adminEnabled \ + --adminListen 10.0.0.1:8080 \ + --s3CredentialsFile=/etc/elastickv/admin-s3-creds.json \ + --adminSessionSigningKeyFile=/etc/elastickv/admin-hs256.b64 \ + --adminFullAccessKeys=AKIA_ADMIN \ + --adminAllowPlaintextNonLoopback \ + --adminAllowInsecureDevCookie \ + --keyvizEnabled \ + --keyvizFanoutNodes=10.0.0.1:8080,10.0.0.2:8080,10.0.0.3:8080 +``` + +The example is the node-1 process. Node 2 and node 3 use their own +reachable `--adminListen` address (`10.0.0.2:8080`, +`10.0.0.3:8080`) while keeping the same symmetric +`--keyvizFanoutNodes` list. `--adminEnabled` is required; without it +the HTTP admin handler is not mounted. The normal admin prerequisites +also still apply: `--s3CredentialsFile` must point at the login +credential map, and all nodes must share the same 64-byte +`--adminSessionSigningKey` through the flag, file, or environment +variable form. At least one role allow-list must also name an access key +from that shared credentials file, for example +`--adminFullAccessKeys=AKIA_ADMIN` or `--adminReadOnlyAccessKeys=...`; +otherwise login succeeds at SigV4 verification but is rejected as not +authorised for admin access. Non-loopback plaintext requires the explicit +`--adminAllowPlaintextNonLoopback` opt-in shown above. A working +plaintext browser rig also requires `--adminAllowInsecureDevCookie`, +because `--adminAllowPlaintextNonLoopback` only permits the listener +bind; it does not remove the `Secure` attribute from admin cookies. +This plaintext form is for short-lived private test rigs. Production +rollouts should use admin TLS and omit both plaintext escape hatches. + +- Self is included implicitly: `internal/admin` matches each + `--keyvizFanoutNodes` entry against `--adminListen` and skips the + network round-trip for the local entry. This keeps the + configuration symmetric across all nodes (every node lists every + node, including itself) so an operator can stamp the same flag + onto every host. The matching rule is: + - **Exact host:port equality** to `--adminListen` is the primary + case (e.g. both say `10.0.0.1:8080`). + - **Wildcard-bind handling**: when the listener bound to + `0.0.0.0` / `::`, an entry with the same port and a loopback + host (`127.0.0.1` / `localhost` / `::1`) also counts as self. + Operators who bind `--adminListen=0.0.0.0:8080` but stamp + `127.0.0.1:8080` into every node's flag still get the + skip-fires behavior. + - **Anything else** is treated as a peer. A reverse-proxy or + DNS-aliased entry that names the same node by a different host + will not match — the fan-out makes a loopback HTTP call to + itself. Because merge semantics sum per-source counters, this can + double-count the local node's read traffic rather than merely add a + wasted round-trip. Operators must either list the literal + `--adminListen` value for self or omit self aliases from + `--keyvizFanoutNodes`. +- Empty (or unset) flag → fan-out disabled, current behaviour. +- Each entry is either `host:port` or a full URL. The host-only form is + interpreted as `http://host:port`. Deployments that enable admin TLS + must use explicit `https://host:port` entries: + `--keyvizFanoutNodes=https://node1.internal:8443,https://node2.internal:8443`. + Full URL entries must be bare `scheme://host:port` values with no + path, query, or trailing slash; self filtering compares the stripped + `host:port` form and treats anything else as a peer. The URL builder + preserves explicit schemes, so TLS fan-out does not require a separate + scheme flag. The Phase 2-C implementation uses the default Go + `http.Client`; HTTPS peer certificates must already chain to the + process or container trust roots. Private-CA or self-signed admin + certificates therefore need that CA installed in the trust store before + fan-out is enabled, otherwise peer calls fail certificate verification + and the cluster view is degraded. +- **Auth (Phase 2-C MVP)**: the aggregator forwards a whitelist of + the inbound user's cookies (`admin_session` + `admin_csrf` only) + on every peer call. The peer's `SessionAuth` middleware verifies + `admin_session` against its own `--adminSessionSigningKey`; + cluster nodes already share that key for HA, so a cookie minted + on node A is verifiable on node B without any new infrastructure. + Unrelated cookies the browser may carry (analytics, feature + flags, other-app sessions on the same domain) are dropped at the + fan-out boundary so they are not leaked across the internal + network (security-medium review finding on PR #692). +- **Recursion guard**: peer requests carry an `X-Admin-Fanout-Peer` + header. The receiving handler short-circuits its own fan-out + when this header is set; without the guard, a symmetric + configuration (every node lists every other node) would generate + O(N²) HTTP calls per browser poll as each peer recursively + fanned out. (P1 review finding on PR #692.) + - The earlier draft of this paragraph said "anonymous on a + private network" — that was wrong: the receiving side enforces + session auth, so anonymous calls are rejected with 401 and the + cluster heatmap collapses to "1 of N nodes responded". The + cookie-forwarding scheme above is what actually works. + - **Trust model**: forwarding a session cookie to peers is safe + when (a) every peer is operator-configured and trusted, and + (b) the network path is private or TLS-protected (cookies are + HttpOnly but still ride the peer request). `--keyvizFanoutNodes` is the + operator's explicit trust list. **Do NOT point + `--keyvizFanoutNodes` at an untrusted host** — the user's + admin session would be replayed there. + - Per-call timeout: 2 s default, override via + `--keyvizFanoutTimeout`. +- **Auth (Phase 2-C+)**: a follow-up replaces cookie forwarding + with a short-lived **inter-node** token derived from + `ELASTICKV_ADMIN_SESSION_SIGNING_KEY`. Pre-shared, NOT a replay + of the browser cookie — re-using the cookie couples browser + session TTL to inter-node call validity, and a compromised + browser session gains peer-call authority. The follow-up + decouples the two paths so the inter-node call survives a + browser-session expiry and a compromised cookie doesn't extend + laterally. + +## 4. Merge rules + +The parent design §9.1 specifies the **canonical** merge rule: +collapse write samples to one value per +`(bucketID, raftGroupID, leaderTerm, windowStart)`, surface a +conflict when distinct sources disagree inside the same identity, +and sum across `leaderTerm` values for the same `(group, window)`. +The implemented JSON and proto rows carry `raft_group_ids` / +`leader_terms` arrays parallel to `values[]`, so leadership can flip +inside the requested window without losing per-column identity. + +### 4.1 Reads → sum across nodes + +Each node records the reads it served. A read served by node A is +not also served by node B. Summation is exact. + +### 4.2 Writes → per-cell group/term dedupe + +Phase 2-C records writes at the ingress node before any follower-to-leader +forwarding. Under direct-to-leader traffic this means one node (the current +leader for the route's group) records writes for a given +`(bucketID, raftGroupID, leaderTerm, column)` while other nodes report 0. +Under follower ingress or multi-ingress clients, multiple nodes can record +the same term locally before the request is forwarded and committed by the +leader. The server therefore treats write cells as ingress-observed samples, +not as an exact leader-only commit counter. Zero-valued contributions are +ignored for write conflict purposes, so follower zeros against a non-zero +value do not perturb the result or raise `conflicts[j]`. + +Under a leadership flip inside the requested window, the ex-leader +and new leader report different `leaderTerm` values in different +columns or even in the same column. The merge: + +- takes max within the same `(bucketID, raftGroupID, leaderTerm, + column)` identity, preserving the pre-existing leader-only fast path + while avoiding double-counting duplicate ingress observations in a + stable term; +- sums the resolved values across distinct `leaderTerm` values for + the same `(bucketID, raftGroupID, column)`, preserving both sides + of a real leadership transition; +- sets `conflicts[j]=true` when two sources report different non-zero + values for the same `(group, term, column)` identity. In Phase 2-C this + can mean either a true same-term sampling anomaly or normal rollout-era + multi-ingress traffic whose local ingress counters differ; the wire + contract intentionally surfaces the ambiguity instead of claiming exact + leader-only write accounting. + +For mixed-version clusters or cells whose identity is unknown +(`raftGroupID == 0` or `leaderTerm == 0`), that cell falls back to +the legacy max-merge. This fallback is deliberately conservative: +it may under-count a transition involving an old peer, but it does +not over-count overlapping unknown and known-term samples. If the +fallback observes multiple distinct non-zero values, it also sets the +per-cell conflict bit because operators are seeing a rollout-era +known/unknown disagreement, not a clean same-term Raft invariant +violation. + +### 4.3 Bytes counters + +`read_bytes` follows `reads` (sum across nodes). `write_bytes` +follows `writes` (the same group/term dedupe, sum-across-terms, and +unknown-identity max fallback). + +### 4.4 Row identity + +Two rows from different nodes belong to the same logical row when +their `BucketID` matches. The serving node merges its local matrix +first, then appends peer matrices, so `Start` / `End` / `Aggregate` / +`RouteCount` / `RouteIDs` are taken from the **serving node's local row +when present**. If the serving node has no row for that `BucketID`, the +first peer row in the fan-out merge order supplies the metadata. This +is intentionally a local-first contract, not a cluster-wide +config-order winner: during routing-catalog divergence, different +serving nodes can expose their own local metadata while the counters +still merge by `BucketID`. If two nodes disagree on `Start`/`End` +for the same `BucketID`, that indicates a routing-catalog +divergence, but Phase 2-C does not surface that condition on the +wire. It is not reported in `FanoutNodeStatus`, row metadata, or a +warning array; the aggregator keeps the local-first metadata winner +and still serves the merged response. Surfacing catalog-divergence +warnings is a future wire extension. + +### 4.5 Time alignment + +All nodes use `keyvizStep` (default 60 s), but the shipped Phase 2-C +aggregator does not receive `keyvizStep` and does not bucket-align +peer column timestamps. It merges exact `column_unix_ms` values from +the local and peer matrices. Two nodes whose flush goroutines fire at +different millisecond offsets therefore produce adjacent merged +columns even when they represent the same logical sampling window. + +Specifics: + +- The aggregator unions exact `column_unix_ms` entries before merging. +- A column present in node A but absent in node B contributes + only A's values for that bucket — B's missing column reads as + zero (the merge-rule identity). +- Bucket-aligning timestamps to `keyvizStep` remains a future + extension. It will require passing the effective step duration into + the fan-out merge path so routine sub-step jitter can be coalesced + without hiding larger clock drift. + +## 5. Wire format + +Response shape (additions to the existing `KeyVizMatrix` only, +no breaking changes — old SPA versions keep working): + +```jsonc +{ + "column_unix_ms": [...], + "rows": [ + { + "bucket_id": "route:42", + ..., + "values": [...], + "conflict": true, // OR of conflicts[] + "conflicts": [false, true, false], + "raft_group_ids": [7, 7, 7], + "leader_terms": [42, 43, 43] + } + ], + "series": "writes", + "generated_at": "...", + + // NEW fan-out metadata; absent when fan-out is disabled. + "fanout": { + "nodes": [ + {"node": "10.0.0.1:8080", "ok": true}, + {"node": "10.0.0.2:8080", "ok": true}, + {"node": "10.0.0.3:8080", "ok": false, "error": "context deadline exceeded"} + ], + "responded": 2, + "expected": 3 + } +} +``` + +`FanoutNodeStatus` currently serializes only `node`, `ok`, and +`error`; `error` is omitted for successful nodes. Structured per-node +warnings such as catalog-divergence are not shipped in Phase 2-C, and +catalog divergence is not encoded anywhere else in the response today. +A future warning array would be an additive wire extension. + +`conflicts[]` is parallel to `values[]` and marks only the columns +where the fan-out merge saw disagreement inside the same +`(bucketID, raftGroupID, leaderTerm, column)` identity or distinct +non-zero values under the unknown-identity fallback. `conflict` is the +row-level OR of `conflicts[]` and remains on the wire for older SPAs +that only know how to hatch a whole row. + +`KeyVizRow.Conflict` is tagged `json:"conflict,omitempty"`, so +absence means `false`. Only the fan-out aggregator sets +`conflict = true` or allocates `conflicts[]`; in local-only mode and +cleanly merged rows, both `conflict` and `conflicts` are omitted. +`raft_group_ids[]` and `leader_terms[]` are omitted only for legacy +peers that do not emit per-column identity. Current JSON responses +still allocate arrays parallel to `values[]`; unknown identity is +represented by zero entries, matching `proto/admin.proto`'s +"term not tracked" sentinel. + +## 6. SPA changes + +- New `Fanout` block in the API client (TypeScript shape mirroring + §5). +- A degraded-mode banner above the heatmap when `responded < + expected`, listing which nodes failed and why. +- `KeyVizRow.conflicts[]` → per-cell hatching. `KeyVizRow.conflict` + remains the row-level fallback for older clients and for any + consumer that wants a cheap row summary. +- Header counter: "Cluster view (3 of 3 nodes)". + +This is small enough to land in the same PR as the server side or +as an immediate follow-up; either way the wire format above is +forwards-compatible so an old SPA against a fan-out server still +renders the merged values (it just ignores the new fields and lacks +the degraded/conflict affordances). + +## 7. Implementation status + +Implemented server slice: + +- `--keyvizFanoutNodes` flag plumbing through `main.go` → + `internal/admin` → handler. +- `KeyVizRow` JSON fields `conflicts`, `raft_group_ids`, and + `leader_terms`, each parallel to `values[]`. +- Fan-out aggregator with the shipped §4 merge rules, degraded peer + status, peer timeouts, bounded peer response bodies, cookie + forwarding, and recursion guard. Timestamp bucket alignment and + structured catalog-divergence warnings remain future extensions. +- Table-driven tests for reads, canonical write merge, unknown-term + fallback, per-cell conflict, partial failure, peer ordering, URL + forwarding, and response-size guard paths. + +Implemented SPA slice: + +- Degraded banner, conflict hatching, and cluster-view header + counter. + +Deferred follow-ups: + +- Dynamic membership discovery via `GetClusterOverview`. +- Inter-node token auth replacing browser-cookie forwarding. + +## 8. Five-lens review checklist + +1. **Data loss** — Fan-out is read-only against the existing + sampler. With per-cell `raftGroupID` / `leaderTerm` identity, the + implemented write merge preserves distinct leadership terms rather + than collapsing them to max. Mixed-version or unknown-identity + cells fall back to max-merge, which may under-count but avoids + inventing traffic; `conflicts[]` tells operators which cells are + soft. +2. **Concurrency / distributed** — Per-node calls are issued in + parallel with the configured per-call timeout (default 2 s, + override via `--keyvizFanoutTimeout` per §3). The aggregator + itself is single-goroutine (waits for all peer responses then + merges). No shared mutable state between concurrent fan-out + requests. +3. **Performance** — Aggregator cost is O(N × M × C) where N is + nodes, M is rows per node, C is columns. At the 1024-row + budget × 3 nodes × 60 columns this is ~180k cells per + request — well below the SPA's existing render budget. No + coordinated round trip per `Observe`; the existing hot path is + not touched. **Per-peer response body is capped at 64 MiB** + via `io.LimitReader` — a misbehaving or compromised peer that + streams gigabytes back at us would otherwise pin a goroutine + on the JSON decoder and balloon memory. Sizing: at the design + ceiling of 1024 rows × 4096 columns × 8 B/uint64 the raw binary + payload is ~32 MiB. For sparse heatmaps where most counters are + ≤ 4 decimal digits (the common case), JSON's decimal text form + is **smaller** than the 8-byte raw binary, so the 64 MiB cap + gives comfortable headroom over the raw-binary ceiling. The cap + does become a real ceiling for traffic-saturated heatmaps where + counters routinely encode to ~20 digits, but those sit well + above the realistic operational range. Beyond that, note that + `MaxHistoryColumns` in the + sampler is **100 000** at the upper bound; a peer that returns + the full ring (~1024 rows × 100 000 cols ≈ 800 MiB raw) would + trip the cap. The implementation distinguishes two cases: + - If a complete JSON object is decoded before the extra capped byte + is consumed (for example, a small object plus trailing whitespace), + the peer remains `ok=true`, the decoded matrix is accepted, and a + `WARN`-level server log records that the response crossed the + configured body limit. + - If the cap cuts through the JSON object, decode returns an error; + the aggregator records that peer as `ok=false` with the decode + error in the node status entry, and no partial data from that peer + is merged. + Operators who actually want >64 MiB peer responses should override + via a future flag; for now the conservative default is the correct + trade. (Review finding on PR #685.) +4. **Data consistency** — Reads are exact in steady state and during + transitions. Writes use the implemented §9.1 group/term merge + when all contributors provide identity, so leadership transitions + preserve both terms rather than collapsing to max. Cells with + legacy or unknown identity fall back to conservative max-merge + to avoid over-counting overlapping known and unknown samples. +5. **Test coverage** — `internal/admin/keyviz_fanout_test.go` + covers canonical group/term merge, unknown-term fallback, + per-cell conflict, row-level conflict OR, reads, partial failure, + URL builder variants, per-node order, and the over-cap path. + Existing handler tests remain unchanged when fan-out is disabled + (the default). The synthetic-burst test at + `keyviz/sampler_test.go` is unaffected. + +## 9. Open questions + +1. Should `--keyvizFanoutNodes` accept hostnames that resolve via + DNS (so a Kubernetes headless service like + `elastickv-admin:8080` works), or require pre-resolved + addresses? **Resolved as: DNS allowed; resolution rides the Go + stdlib resolver's process-wide cache plus the kernel's nscd + cache when configured — no resolver cache local to the + aggregator.** A transient DNS failure surfaces as a per-peer + `ok=false`, not a hung request, because the `http.Client` honors + the per-call deadline. Operators on networks with flaky DNS + should colocate a caching resolver (the standard fix for any + short-lived HTTP client, not specific to this feature). + (Round-1 review finding on PR #685.) +2. **Resolved**: per-node call budget is a fixed 2 s default with + `--keyvizFanoutTimeout` operator override. Tying the timeout to + `keyvizStep` would let a 60-second-step config hold the request + for ~2 minutes against a slow peer; the fixed default decouples + the fan-out wall time from the sampling cadence. See §3. +3. Should the fan-out response include each node's `generated_at` + so the SPA can detect skew? Probably yes — adds one timestamp + per node entry, no real cost. Defer the SPA UI to PR 2. diff --git a/docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md b/docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md deleted file mode 100644 index 6e1b8f762..000000000 --- a/docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md +++ /dev/null @@ -1,441 +0,0 @@ ---- -status: proposed -phase: 2-C -parent_design: docs/admin_ui_key_visualizer_design.md -date: 2026-04-27 ---- - -# KeyViz Cluster Fan-out (Phase 2-C) - -## 1. Background - -Phase 2-A (server-side sampler) and Phase 2-B (SPA heatmap) ship a -heatmap that shows **only the local node's view**. Per the parent -design §5.1 reads are recorded by the follower that serves them and -writes are recorded by the leader; pointing the SPA at a single node -therefore shows a partial picture — operators looking at a follower -see no writes, operators looking at a leader see only that leader's -writes. - -Parent design §9.1 defines the desired end state: the admin layer -fans out to every node, merges responses with rules that respect -Raft leadership, and renders one combined heatmap with degraded-node -status surfaced inline. - -**Rollout notes** (Gemini round-1 PR #685 asked for explicit -rolling-upgrade and zero-downtime cutover plans). This change is -**off by default** (`--keyvizFanoutNodes` empty) and lives entirely -on the admin path — no data-plane impact, no schema migration, no -state stored on disk by this feature. Mixed-version clusters mid- -rollout are correct by construction: - -- Wire format additions (§5) are forwards-compatible: an old node - that does not know about `Fanout` / `conflict` simply omits them - and the new aggregator treats the missing values as the merge-rule - identity. An old SPA against a new server ignores the extra - fields and renders the local view it always did. -- Operators flip `--keyvizFanoutNodes` per-node during the rollout. - Until every node has the new binary AND the flag set, some peer - HTTP calls will 404 — that surfaces as `ok=false` in the - per-node status array, not as a failed user request. -- No dual-write proxy / blue-green required because there is no - state to migrate; the worst-case partial deploy is "node X - reports `ok=false`", not data loss. - -This proposal scopes a **minimum-viable Phase 2-C** that ships the -operator-visible value (cluster-wide -heatmap, degraded-node banner) without requiring the full proto -extension §9.1 calls for. - -## 2. Scope - -### 2.1 In scope - -- A static node list configured via `--keyvizFanoutNodes` (comma- - separated admin HTTP endpoints). -- An aggregator inside `internal/admin` that issues parallel GETs to - every configured node's `/admin/api/v1/keyviz/matrix`, merges the - responses, and returns one combined `KeyVizMatrix` plus a per-node - status array. -- Two merge rules, justified in §4: - - **Reads**: sum across nodes (each node serves distinct local - follower reads). - - **Writes**: max across nodes, with a row-level `conflict=true` - flag raised when **two or more nodes report a non-zero value - for the same cell** in the row. Follower zeros against the - leader's non-zero value do **not** count as disagreement — - that is the steady-state shape of stable leadership. Correct - under stable leadership (the current leader is the sole - non-zero reporter; conflict stays false), conservative under a - leadership flip mid-window (both leaders report > 0 for the - same cell; max wins and conflict fires). The flag is row-level - for Phase 2-C and moves to per-cell when the proto extension - lands in Phase 2-C+ — see §4.2 and §5 for the wire-format - implication. -- Degraded-mode response: when N nodes respond and M < N succeed, - the response carries `{node, ok, error}` per node and the SPA - shows a banner. -- Behaviour preserved when `--keyvizFanoutNodes` is unset: the - endpoint serves the local view exactly as it does today. - -### 2.2 Explicitly NOT in scope - -- **Membership discovery via `GetClusterOverview`** (parent §9.1). - Static `--keyvizFanoutNodes` is the contract for Phase 2-C; the - cache + refresh-interval machinery lands when the cluster grows - beyond a hand-configured size. -- **Wire format extension with `raftGroupID` + `leaderTerm`** - (parent §9.1). The MVP merge uses `max` over writes; the - identity-based dedup §9.1 specifies will land in Phase 2-C+ when - we extend the proto + JSON schemas. -- **Standalone `cmd/elastickv-admin` binary** (parent §3.1). - Deferred indefinitely — we have not seen a use case the embedded - in-node admin does not cover, and this design holds the - embedded path as the go-forward. -- **Response-payload caching across browser polls.** Each request - fans out fresh. Coalescing concurrent requests is a tomorrow - problem if it shows up in benchmarks. - -## 3. Configuration - -```sh -elastickv \ - --address 127.0.0.1:50051 \ - --adminAddress 127.0.0.1:8080 \ - --keyvizEnabled \ - --keyvizFanoutNodes=10.0.0.1:8080,10.0.0.2:8080,10.0.0.3:8080 -``` - -- Self is included implicitly: `internal/admin` matches each - `--keyvizFanoutNodes` entry against `--adminAddress` and skips the - network round-trip for the local entry. This keeps the - configuration symmetric across all nodes (every node lists every - node, including itself) so an operator can stamp the same flag - onto every host. The matching rule is: - - **Exact host:port equality** to `--adminAddress` is the primary - case (e.g. both say `10.0.0.1:8080`). - - **Wildcard-bind handling**: when the listener bound to - `0.0.0.0` / `::`, an entry with the same port and a loopback - host (`127.0.0.1` / `localhost` / `::1`) also counts as self. - Operators who bind `--adminAddress=0.0.0.0:8080` but stamp - `127.0.0.1:8080` into every node's flag still get the - skip-fires behavior. - - **Anything else** is treated as a peer. A reverse-proxy or - DNS-aliased entry that names the same node by a different host - will not match — the fan-out makes a loopback HTTP call to - itself. This is harmless (it degrades to one extra round-trip - per request) but wasteful; operators should prefer the literal - `--adminAddress` value in the flag. -- Empty (or unset) flag → fan-out disabled, current behaviour. -- Each entry is a host:port. TLS is out of scope for Phase 2-C - (intra-cluster admin traffic is assumed to ride a private - network); the HTTP client uses `http://`. A follow-up will - introduce `--keyvizFanoutTLS` once the rest of the admin path - has TLS too. -- **Auth (Phase 2-C MVP)**: the aggregator forwards a whitelist of - the inbound user's cookies (`admin_session` + `admin_csrf` only) - on every peer call. The peer's `SessionAuth` middleware verifies - `admin_session` against its own `--adminSessionSigningKey`; - cluster nodes already share that key for HA, so a cookie minted - on node A is verifiable on node B without any new infrastructure. - Unrelated cookies the browser may carry (analytics, feature - flags, other-app sessions on the same domain) are dropped at the - fan-out boundary so they are not leaked across the internal - network (Gemini security-medium on PR #692). -- **Recursion guard**: peer requests carry an `X-Admin-Fanout-Peer` - header. The receiving handler short-circuits its own fan-out - when this header is set; without the guard, a symmetric - configuration (every node lists every other node) would generate - O(N²) HTTP calls per browser poll as each peer recursively - fanned out. (Claude bot P1 on PR #692.) - - The earlier draft of this paragraph said "anonymous on a - private network" — that was wrong: the receiving side enforces - session auth, so anonymous calls are rejected with 401 and the - cluster heatmap collapses to "1 of N nodes responded". The - cookie-forwarding scheme above is what actually works. - - **Trust model**: forwarding a session cookie to peers is safe - when (a) every peer is operator-configured and trusted, and - (b) the network is private (cookies are HttpOnly but ride - plaintext HTTP for now). `--keyvizFanoutNodes` is the - operator's explicit trust list. **Do NOT point - `--keyvizFanoutNodes` at an untrusted host** — the user's - admin session would be replayed there. - - Per-call timeout: 2 s default, override via - `--keyvizFanoutTimeout`. -- **Auth (Phase 2-C+)**: a follow-up replaces cookie forwarding - with a short-lived **inter-node** token derived from - `ELASTICKV_ADMIN_SESSION_SIGNING_KEY`. Pre-shared, NOT a replay - of the browser cookie — re-using the cookie couples browser - session TTL to inter-node call validity, and a compromised - browser session gains peer-call authority. The follow-up - decouples the two paths so the inter-node call survives a - browser-session expiry and a compromised cookie doesn't extend - laterally. - -## 4. Merge rules - -The parent design §9.1 specifies the **canonical** merge rule: -collapse write samples to one value per -`(bucketID, raftGroupID, leaderTerm, windowStart)`, surface -`conflict=true` when distinct sources disagree, sum across -`leaderTerm` values for the same `(group, window)`. That rule -requires `raftGroupID` + `leaderTerm` on every row — proto + JSON -extensions we have not built yet. - -This MVP uses a **simpler, conservative substitute** that is -correct in steady state and never silently overcounts under -transitions: - -### 4.1 Reads → sum across nodes - -Each node records the reads it served. A read served by node A is -not also served by node B. Summation is exact. - -### 4.2 Writes → max across nodes + conflict flag - -Under stable leadership, exactly one node (the current leader for -the route's group) records writes for any given window; other -nodes report 0. `max` returns the leader's count; non-leader -zeros do not perturb it **and do not raise the conflict flag**. -The conflict predicate is "≥ 2 nodes report a value > 0 for the -same cell", not "any two values differ" — a leader-says-5 vs. -follower-says-0 split is the steady-state shape, not a conflict. - -Under a leadership flip mid-window, both the ex-leader and the -new leader may report non-zero counts for the same -`(bucketID, windowStart)`: - -- If the ex-leader's local cache has not yet expired its - pre-transfer increment, both nodes report values > 0 for the - same cell. The conflict predicate fires. -- `max` returns the larger count and surfaces `conflict=true` so - the SPA can hatch the cell. -- This is **conservative** — it understates the true window - total (which is `ex_leader + new_leader`) by at most one - leader's pre-transfer slice. The design accepts this trade - in exchange for not overcounting; overcounting is the failure - mode operators react to badly because it produces phantom - hotspots. - -The full §9.1 rule (sum across `leaderTerm`, dedupe within a -term) recovers the missing slice but requires `leaderTerm` on -the wire. We commit to landing it in Phase 2-C+ once the proto -extension lands. - -### 4.3 Bytes counters - -`read_bytes` and `write_bytes` follow `reads` and `writes` -respectively (sum and max). Same justification. - -### 4.4 Row identity - -Two rows from different nodes belong to the same logical row when -their `BucketID` matches. After the aggregator has collected **all** -per-node responses (i.e. waited for every peer's deadline to -resolve), `Start` / `End` / `Aggregate` / `RouteCount` / `RouteIDs` -are taken from the **lowest-indexed node in `--keyvizFanoutNodes` -whose response is non-empty for the row**. Selection by config-list -position rather than wall-clock arrival order is the load-bearing -property: peers respond in non-deterministic order, so picking on -first arrival would let `Start`/`End` flap between polls for the -same bucket. If two nodes disagree on `Start`/`End` -for the same `BucketID`, that indicates a routing-catalog -divergence the operator should investigate; we surface it as a -warning in the per-node status payload but do not block the -response. - -### 4.5 Time alignment - -All nodes use `keyvizStep` (default 1 s). The aggregator **bucket- -aligns** column timestamps to the nearest `keyvizStep` boundary -before pivoting, so two nodes whose flush goroutines fire fractions -of a second apart still land on the same merged column. Without -the alignment step a 50 ms NTP skew would split each window into -two adjacent merged columns, halving the displayed traffic in each. - -In the rules below, `keyvizStep_ms` denotes the step duration in -milliseconds — i.e. `time.Duration(--keyvizStep) / time.Millisecond`. -The wire form already carries timestamps as `column_unix_ms` so -millisecond-quantum arithmetic falls out naturally. - -Specifics: - -- Both sides round `column_unix_ms[i]` down to the nearest - multiple of `keyvizStep_ms` before merging. -- A column present in node A but absent in node B contributes - only A's values for that bucket — B's missing column reads as - zero (the merge-rule identity). -- Drift larger than `keyvizStep / 2` between nodes is still an - operator problem (the heatmap should not paper over NTP issues - that big), but routine sub-step jitter no longer causes column - fragmentation. (Gemini round-1 PR #685.) - -## 5. Wire format - -Response shape (additions to the existing `KeyVizMatrix` only, -no breaking changes — old SPA versions keep working): - -```jsonc -{ - "column_unix_ms": [...], - "rows": [ - { - "bucket_id": "route:42", - ..., - "values": [...], - "conflict": false // NEW: true when MVP max-merge resolved disagreement - } - ], - "series": "writes", - "generated_at": "...", - - // NEW fan-out metadata; absent when fan-out is disabled. - "fanout": { - "nodes": [ - {"node": "10.0.0.1:8080", "ok": true, "error": "", "warnings": []}, - {"node": "10.0.0.2:8080", "ok": true, "error": "", - "warnings": [{"code": "catalog_divergence", "bucket_id": "route:42", - "detail": "start/end disagree with prior node"}]}, - {"node": "10.0.0.3:8080", "ok": false, "error": "context deadline exceeded", "warnings": []} - ], - "responded": 2, - "expected": 3 - } -} -``` - -`warnings` is a per-node array of structured non-fatal signals. -Today the only emitter is `catalog_divergence` (§4.4 — `Start`/`End` -disagree across nodes for the same `BucketID`). Adding a new -warning code is a wire-format extension, not a breaking change: -old SPAs render the entry as a generic warning until they teach -themselves the new code. (Codex round-1 PR #685.) - -`conflict` is per row — a coarser signal than parent §9.1's -per-cell flag. The cell-level flag will land with the -`leaderTerm`-based merge. - -The field is **always present** on every row in the response — even -when fan-out is disabled (single-node mode), `conflict` defaults to -`false` and is emitted on the wire. This is wire-stable: an SPA -that pattern-matches `"conflict" in row` rather than checking the -value will not regress when the operator toggles -`--keyvizFanoutNodes`. Only the fan-out aggregator ever sets -`conflict = true`; in local-only mode the field's value is -identically false. - -## 6. SPA changes - -- New `Fanout` block in the API client (TypeScript shape mirroring - §5). -- A degraded-mode banner above the heatmap when `responded < - expected`, listing which nodes failed and why. -- `KeyVizRow.conflict` → cell hatching (an SVG overlay layered - over the canvas, only over rows whose `conflict === true`). The - hatch is per-row, not per-cell, until the wire format upgrade - lets us be precise. -- Header counter: "Cluster view (3 of 3 nodes)". - -This is small enough to land in the same PR as the server side or -as an immediate follow-up; either way the wire format above is -forwards-compatible so an old SPA against a fan-out server still -renders correctly (it just ignores the new fields). - -## 7. Implementation plan - -PR 1 (this proposal + a small slice): - -- Land this design doc. -- Wire `--keyvizFanoutNodes` flag plumbing through `main.go` → - `internal/admin` → handler. -- Add a fan-out aggregator with the §4 merge rules and a - table-driven test covering: stable-leader (max wins, no - conflict), mid-window flip (max wins, conflict=true), partial - failure (one node times out, response carries the failure). -- Server-side only — SPA changes follow. - -PR 2: - -- SPA: degraded banner, conflict hatching, header counter. - -PR 3 (Phase 2-C+): - -- Extend proto + JSON with `raftGroupID` and `leaderTerm`. -- Replace §4.2 max-merge with the canonical - `(bucketID, raftGroupID, leaderTerm, windowStart)` merge. -- Promote `conflict` from per-row to per-cell. - -## 8. Five-lens review checklist - -1. **Data loss** — Fan-out is read-only against the existing - sampler. The conservative max-merge can under-count during a - leadership flip; this is preferable to over-counting (which - would invent traffic) and the conflict flag tells operators - when the data is soft. -2. **Concurrency / distributed** — Per-node calls are issued in - parallel with the configured per-call timeout (default 2 s, - override via `--keyvizFanoutTimeout` per §3). The aggregator - itself is single-goroutine (waits for all peer responses then - merges). No shared mutable state between concurrent fan-out - requests. -3. **Performance** — Aggregator cost is O(N × M × C) where N is - nodes, M is rows per node, C is columns. At the 1024-row - budget × 3 nodes × 60 columns this is ~180k cells per - request — well below the SPA's existing render budget. No - coordinated round trip per `Observe`; the existing hot path is - not touched. **Per-peer response body is capped at 64 MiB** - via `io.LimitReader` — a misbehaving or compromised peer that - streams gigabytes back at us would otherwise pin a goroutine - on the JSON decoder and balloon memory. Sizing: at the design - ceiling of 1024 rows × 4096 columns × 8 B/uint64 the raw binary - payload is ~32 MiB. For sparse heatmaps where most counters are - ≤ 4 decimal digits (the common case), JSON's decimal text form - is **smaller** than the 8-byte raw binary, so the 64 MiB cap - gives comfortable headroom over the raw-binary ceiling. The cap - does become a real ceiling for traffic-saturated heatmaps where - counters routinely encode to ~20 digits, but those sit well - above the realistic operational range. Beyond that, note that - `MaxHistoryColumns` in the - sampler is **100 000** at the upper bound; a peer that returns - the full ring (~1024 rows × 100 000 cols ≈ 800 MiB raw) would - trip the cap. The cap firing surfaces as a JSON-decode error - from the peer (the LimitReader returns EOF mid-stream), which - the aggregator records as `ok=false` with the decode error in - that node's status entry plus a `WARN`-level server log. No - partial data is accepted. Operators who actually want >64 MiB - peer responses should override via a future flag; for now the - conservative default is the correct trade. (Gemini PR #685.) -4. **Data consistency** — Merge rules are conservative under - leadership transitions (under-count + conflict flag, never - over-count). Reads are exact in steady state and during - transitions. The §9.1 canonical rule is preserved as a Phase - 2-C+ contract once we extend the wire format. -5. **Test coverage** — `internal/admin/keyviz_fanout_test.go` - table-driven across the §7 PR-1 scenarios (stable-leader, - leadership-flip, partial failure) plus url-builder variants, - per-node order, and the over-cap path. Existing handler tests - unchanged when fan-out is disabled (the default). The - synthetic-burst test at `keyviz/sampler_test.go` is unaffected. - -## 9. Open questions - -1. Should `--keyvizFanoutNodes` accept hostnames that resolve via - DNS (so a Kubernetes headless service like - `elastickv-admin:8080` works), or require pre-resolved - addresses? **Resolved as: DNS allowed; resolution rides the Go - stdlib resolver's process-wide cache plus the kernel's nscd - cache when configured — no resolver cache local to the - aggregator.** A transient DNS failure surfaces as a per-peer - `ok=false`, not a hung request, because the `http.Client` honors - the per-call deadline. Operators on networks with flaky DNS - should colocate a caching resolver (the standard fix for any - short-lived HTTP client, not specific to this feature). - (Gemini round-1 PR #685.) -2. **Resolved**: per-node call budget is a fixed 2 s default with - `--keyvizFanoutTimeout` operator override. Tying the timeout to - `keyvizStep` would let a 60-second-step config hold the request - for ~2 minutes against a slow peer; the fixed default decouples - the fan-out wall time from the sampling cadence. See §3. -3. Should the fan-out response include each node's `generated_at` - so the SPA can detect skew? Probably yes — adds one timestamp - per node entry, no real cost. Defer the SPA UI to PR 2. diff --git a/docs/design/2026_04_28_implemented_keyviz_adapter_labels.md b/docs/design/2026_04_28_implemented_keyviz_adapter_labels.md index 831f8cd18..2fec86c53 100644 --- a/docs/design/2026_04_28_implemented_keyviz_adapter_labels.md +++ b/docs/design/2026_04_28_implemented_keyviz_adapter_labels.md @@ -587,7 +587,7 @@ from PR #694: Claude bot critical, Gemini high.) The fan-out aggregator's per-cell merge key gains the label: - Phase 2-C (current): `(bucketID, raftGroupID, leaderTerm, - windowStart)` per design `2026_04_27_proposed_keyviz_cluster_fanout.md` + windowStart)` per design `2026_04_27_implemented_keyviz_cluster_fanout.md` §4. - With labels: same tuple — but `bucketID` itself now carries the label via the §5 composite (`route:1:dynamo`). The merge key @@ -595,12 +595,15 @@ The fan-out aggregator's per-cell merge key gains the label: encoded into `bucketID` so the aggregator already separates same-route different-label rows correctly. -Reads still sum, writes still max-with-conflict; nothing about -the merge **rules** changes other than the wire shape of -`bucketID`. The merge **key** logic is unchanged: composite -`bucketID` already separates same-route different-label rows -correctly, so the aggregator's bucketing/grouping path needs no -edits. +The merge **rules** do not change. Phase 2-C+ already uses the +canonical per-cell identity `(bucketID, raftGroupID, leaderTerm, +column)`: read samples sum across nodes, write samples are deduped +within the same `(bucketID, raftGroupID, leaderTerm, column)` and +summed across distinct terms, and conflicts are marked only for +same-identity disagreements. Adapter labels only change the +`bucketID` dimension by embedding the label into the composite ID, so +same-route different-label rows are already separated by the existing +bucketing/grouping path. The merge **value** path, however, still needs one explicit field copy: `mergeRowInto` (`internal/admin/keyviz_fanout.go:509`) diff --git a/docs/design/2026_04_29_proposed_logical_backup.md b/docs/design/2026_04_29_proposed_logical_backup.md index 24c033cd8..246b98584 100644 --- a/docs/design/2026_04_29_proposed_logical_backup.md +++ b/docs/design/2026_04_29_proposed_logical_backup.md @@ -1568,9 +1568,10 @@ Scope: out of this proposal; mentioned only to draw the boundary. between two `read_ts` values and dumps the delta in the same per-adapter format under a `delta/-/` subtree. - Concurrent multi-cluster fan-out (one logical backup spanning shards - on different physical clusters) — depends on the `Distribution` - control plane being fan-out-aware (see - `2026_04_27_proposed_keyviz_cluster_fanout.md`). + on different physical clusters) — depends on a future + `Distribution` control-plane fan-out and membership-discovery design. + The implemented KeyViz fan-out is a static admin-HTTP heatmap + aggregator, not the backup discovery contract. ## Required Tests diff --git a/docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md b/docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md index 2f8a1c799..89c2e85a4 100644 --- a/docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md +++ b/docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md @@ -161,7 +161,7 @@ Options considered: **Follower-forwarded write caveat (codex P2).** `ShardedCoordinator.observeMutation` runs on the node that received the client request before `ShardRouter.Commit` forwards (`kv/sharded_coordinator.go:1795-1844`), and the follower's `Internal.Forward` handler calls `transactionManager.Commit` directly (`adapter/internal.go:52`) rather than re-entering the leader's `ShardedCoordinator` observation hook. Therefore even a route whose shard group is led by the default-group leader can be **under-observed** if most clients connect through followers. M3 does not treat "missing local rows" as evidence of low load: local evidence can promote a split, but absence of local evidence only means "unknown" and is fail-closed for target selection (§6.2) and conservative for candidacy (fewer splits). Full coverage for follower-forwarded writes needs follower reports / fan-out into the same detector interface and is tracked under OQ-5. -**Why not the keyviz cluster fan-out** (`docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md`): that aggregates across nodes for the admin heatmap UI and dedupes write samples by `(groupID, leaderTerm)`. Pulling cross-node data in would add latency and a dependency on the fan-out being configured, so M3 stays leader-local and fail-closed on unknown load. If a future milestone wants complete cluster-wide scoring, including follower-forwarded writes, it can read the same fan-out aggregate behind the same detector interface (§5) — recorded as OQ-5. +**Why not the keyviz cluster fan-out** (`docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md`): that aggregates across nodes for the admin heatmap UI and dedupes write samples by `(groupID, leaderTerm)`. Pulling cross-node data in would add latency and a dependency on the fan-out being configured, so M3 stays leader-local and fail-closed on unknown load. If a future milestone wants complete cluster-wide scoring, including follower-forwarded writes, it can read the same fan-out aggregate behind the same detector interface (§5) — recorded as OQ-5. **Known limitation — multi-leader (multi-group) deployments.** Leader-local consumption is only authoritative for routes whose shard group is led by the **same node** that runs the detector — i.e. the default-group leader (where the scheduler must run, §6.1). In a multi-group cluster (`--raftGroups` with G1, G2, …), the default-group leader is **not necessarily** the leader of G1/G2/…: each node runs a single `ShardedCoordinator` and `MemSampler` and only `Observe`s traffic it *receives* (`kv/sharded_coordinator.go:1795-1824`). Concretely, in a 3-node cluster where node A leads the default group and node B leads G1, **node A's sampler has no data for routes in G1 served through node B**, so the detector on A cannot score or split them. This is **broader than "heavy follower-forwarded reads"**: it is a structural gap for any route whose group leader differs from the default-group leader, not just an edge case of read forwarding. diff --git a/docs/design/2026_06_23_proposed_scaling_roadmap.md b/docs/design/2026_06_23_proposed_scaling_roadmap.md index b51827adb..097b14c56 100644 --- a/docs/design/2026_06_23_proposed_scaling_roadmap.md +++ b/docs/design/2026_06_23_proposed_scaling_roadmap.md @@ -259,8 +259,8 @@ memory each group's private cache/memtable pins. - **keyviz** — the per-route load sampler is wired and allocation-free on the write hot path (`observeMutation`, `kv/sharded_coordinator.go:1841-1846`), - with proposed extensions for cluster fan-out - (`2026_04_27_proposed_keyviz_cluster_fanout.md`), + with implemented extensions for cluster fan-out + (`2026_04_27_implemented_keyviz_cluster_fanout.md`), subrange sampling (`2026_05_25_implemented_keyviz_subrange_sampling.md`), hot-key top-K (`2026_05_28_implemented_keyviz_hot_key_topk.md`), and per-cell conflict (implemented). It is the detection signal M3 reuses. Current diff --git a/internal/admin/keyviz_fanout.go b/internal/admin/keyviz_fanout.go index 472c2fc3f..6e3dfa7af 100644 --- a/internal/admin/keyviz_fanout.go +++ b/internal/admin/keyviz_fanout.go @@ -71,7 +71,7 @@ const keyVizMergeBucketHint = 64 // a stable row order; Responded counts ok=true entries; Expected is // the configured peer count plus self. // -// See docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md 5. +// See docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md 5. type FanoutResult struct { Nodes []FanoutNodeStatus `json:"nodes"` Responded int `json:"responded"` diff --git a/internal/admin/keyviz_handler.go b/internal/admin/keyviz_handler.go index d4192d693..db54f812b 100644 --- a/internal/admin/keyviz_handler.go +++ b/internal/admin/keyviz_handler.go @@ -56,7 +56,7 @@ const keyVizRowBudgetCap = 1024 // // Fanout is non-nil when the handler is configured for cluster-wide // fan-out (Phase 2-C): it carries per-node status so the SPA can -// surface degraded responses inline (see design 2026_04_27_proposed_keyviz_cluster_fanout.md). +// surface degraded responses inline (see design 2026_04_27_implemented_keyviz_cluster_fanout.md). // The field is omitted from the wire form when fan-out is disabled // so old clients keep working unchanged. type KeyVizMatrix struct { diff --git a/main.go b/main.go index f9435cdc1..9fec3f9c3 100644 --- a/main.go +++ b/main.go @@ -236,7 +236,7 @@ var ( // HTTP endpoints (host:port or scheme://host:port). When set, // the admin keyviz handler aggregates the local matrix with // peer responses; when empty, behaviour is unchanged - // (single-node view). See docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md. + // (single-node view). See docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md. keyvizFanoutNodes = flag.String("keyvizFanoutNodes", "", "Comma-separated peer admin endpoints (host:port) for keyviz cluster-wide fan-out; empty disables") keyvizFanoutTimeout = flag.Duration("keyvizFanoutTimeout", keyvizFanoutDefaultTimeout, "Per-peer timeout for keyviz fan-out HTTP calls") ) diff --git a/scripts/rolling-update.env.example b/scripts/rolling-update.env.example index 6ee72dd60..ed5e63f12 100644 --- a/scripts/rolling-update.env.example +++ b/scripts/rolling-update.env.example @@ -116,13 +116,14 @@ ADMIN_ENABLED="false" # merges matrices from each peer so the dashboard renders a cluster- # wide heatmap regardless of which node served the request. The # aggregator forwards the operator's session cookie to each peer -# (PR #692), so peers running with --adminEnabled accept the fan-out -# call as long as the cookie is valid on every node — i.e. the same -# admin signing key (ADMIN_SESSION_SIGNING_KEY_FILE) and matching -# role allow-lists must be configured cluster-wide. Peers without -# --adminEnabled expose an unauthenticated keyviz endpoint and -# respond unconditionally. -# See docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md for the +# (PR #692), so every peer in the list must run with --adminEnabled; +# when admin is disabled the KeyViz handler is not mounted. The cookie +# must be valid on every node — i.e. the same admin signing key +# (ADMIN_SESSION_SIGNING_KEY_FILE) and matching role allow-lists must +# be configured cluster-wide. TLS admin listeners must be listed as +# explicit https://host:port entries; host:port entries are treated as +# http://host:port. +# See docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md for the # full design. KEYVIZ_ENABLED="false" # KEYVIZ_FANOUT_NODES="10.0.0.1:8080,10.0.0.2:8080,10.0.0.3:8080"