From 1743981d12f53408f89ce9b400ca5d063756b0cc Mon Sep 17 00:00:00 2001 From: bootjp Date: Tue, 7 Jul 2026 16:26:36 +0900 Subject: [PATCH 01/17] docs: mark keyviz cluster fanout implemented --- ...4_27_implemented_keyviz_cluster_fanout.md} | 20 +++++++++---------- ...26_04_28_proposed_keyviz_adapter_labels.md | 2 +- .../2026_04_29_proposed_logical_backup.md | 2 +- ...sed_hotspot_split_milestone3_automation.md | 2 +- .../2026_06_23_proposed_scaling_roadmap.md | 2 +- internal/admin/keyviz_fanout.go | 2 +- internal/admin/keyviz_handler.go | 2 +- main.go | 2 +- scripts/rolling-update.env.example | 2 +- 9 files changed, 18 insertions(+), 18 deletions(-) rename docs/design/{2026_04_27_proposed_keyviz_cluster_fanout.md => 2026_04_27_implemented_keyviz_cluster_fanout.md} (97%) diff --git a/docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md similarity index 97% rename from docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md rename to docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index 6e1b8f762..8eae624b6 100644 --- a/docs/design/2026_04_27_proposed_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -1,5 +1,5 @@ --- -status: proposed +status: implemented phase: 2-C parent_design: docs/admin_ui_key_visualizer_design.md date: 2026-04-27 @@ -22,7 +22,7 @@ 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 +**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 @@ -42,7 +42,7 @@ rollout are correct by construction: 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 +This design 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. @@ -141,13 +141,13 @@ elastickv \ 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). + 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. (Claude bot P1 on PR #692.) + 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 @@ -268,7 +268,7 @@ Specifics: - 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.) + fragmentation. (Round-1 review finding on PR #685.) ## 5. Wire format @@ -309,7 +309,7 @@ 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.) +themselves the new code. (Round-1 review finding on 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 @@ -343,7 +343,7 @@ renders correctly (it just ignores the new fields). ## 7. Implementation plan -PR 1 (this proposal + a small slice): +PR 1 (this design + a small slice): - Land this design doc. - Wire `--keyvizFanoutNodes` flag plumbing through `main.go` → @@ -404,7 +404,7 @@ PR 3 (Phase 2-C+): 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.) + conservative default is the correct trade. (Review finding on 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 @@ -430,7 +430,7 @@ PR 3 (Phase 2-C+): 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.) + (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 diff --git a/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md b/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md index 88ad5e4ef..d760e87ff 100644 --- a/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md +++ b/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md @@ -579,7 +579,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 diff --git a/docs/design/2026_04_29_proposed_logical_backup.md b/docs/design/2026_04_29_proposed_logical_backup.md index 24c033cd8..39350d188 100644 --- a/docs/design/2026_04_29_proposed_logical_backup.md +++ b/docs/design/2026_04_29_proposed_logical_backup.md @@ -1570,7 +1570,7 @@ Scope: out of this proposal; mentioned only to draw the boundary. - 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`). + `2026_04_27_implemented_keyviz_cluster_fanout.md`). ## Required Tests diff --git a/docs/design/2026_06_11_proposed_hotspot_split_milestone3_automation.md b/docs/design/2026_06_11_proposed_hotspot_split_milestone3_automation.md index 6636f3f7a..7d6045b99 100644 --- a/docs/design/2026_06_11_proposed_hotspot_split_milestone3_automation.md +++ b/docs/design/2026_06_11_proposed_hotspot_split_milestone3_automation.md @@ -142,7 +142,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 91afcdfcf..e8c26cae3 100644 --- a/docs/design/2026_06_23_proposed_scaling_roadmap.md +++ b/docs/design/2026_06_23_proposed_scaling_roadmap.md @@ -269,7 +269,7 @@ 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`), + (`2026_04_27_implemented_keyviz_cluster_fanout.md`), subrange sampling (`2026_05_25_proposed_keyviz_subrange_sampling.md`), hot-key top-K (`2026_05_28_proposed_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 3a245f67a..431f19807 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 778fe6054..ae672e8b9 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 742af112d..e3eaf81f4 100644 --- a/main.go +++ b/main.go @@ -211,7 +211,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 d47805b0e..e6bda0427 100644 --- a/scripts/rolling-update.env.example +++ b/scripts/rolling-update.env.example @@ -122,7 +122,7 @@ ADMIN_ENABLED="false" # 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 +# 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" From dfc61d341fd06144e4026053e38f677f36894ac7 Mon Sep 17 00:00:00 2001 From: bootjp Date: Tue, 7 Jul 2026 16:37:58 +0900 Subject: [PATCH 02/17] docs: update keyviz phase status --- docs/admin_ui_key_visualizer_design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index 2f296a6a1..e79a1f787 100644 --- a/docs/admin_ui_key_visualizer_design.md +++ b/docs/admin_ui_key_visualizer_design.md @@ -325,7 +325,7 @@ Because writes are recorded by Raft leaders and follower-local reads are recorde | 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 From 32a6e4f27b9002a75cb29911fcb06df43be077a4 Mon Sep 17 00:00:00 2001 From: bootjp Date: Wed, 8 Jul 2026 15:56:41 +0900 Subject: [PATCH 03/17] docs: align keyviz fanout spec --- ...04_27_implemented_keyviz_cluster_fanout.md | 204 +++++++++--------- 1 file changed, 101 insertions(+), 103 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index 8eae624b6..e910a61df 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -42,10 +42,9 @@ rollout are correct by construction: state to migrate; the worst-case partial deploy is "node X reports `ok=false`", not data loss. -This design 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. +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 @@ -60,18 +59,15 @@ extension §9.1 calls for. - 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. + - **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)`. + 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. @@ -80,14 +76,10 @@ extension §9.1 calls for. ### 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. +- **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 @@ -176,56 +168,51 @@ elastickv \ 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: +`(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 → max across nodes + conflict flag +### 4.2 Writes → per-cell group/term dedupe 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. +the route's group) records writes for a given +`(bucketID, raftGroupID, leaderTerm, column)`; other nodes report +0. Zero-valued contributions are ignored for write conflict +purposes, so follower zeros against the leader's 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, because Raft has at most one leader per group + and 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` only when two sources report different + non-zero values for the same `(group, term, column)` identity. + +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. ### 4.3 Bytes counters -`read_bytes` and `write_bytes` follow `reads` and `writes` -respectively (sum and max). Same justification. +`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 @@ -283,7 +270,10 @@ no breaking changes — old SPA versions keep working): "bucket_id": "route:42", ..., "values": [...], - "conflict": false // NEW: true when MVP max-merge resolved disagreement + "conflict": false, // OR of conflicts[] + "conflicts": [false, true, false], + "raft_group_ids": [7, 7, 7], + "leader_terms": [42, 43, 43] } ], "series": "writes", @@ -311,9 +301,11 @@ 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. (Round-1 review finding on 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. +`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. `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. The field is **always present** on every row in the response — even when fan-out is disabled (single-node mode), `conflict` defaults to @@ -321,8 +313,10 @@ when fan-out is disabled (single-node mode), `conflict` defaults to 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. +`conflict = true` or allocates `conflicts[]`; in local-only mode +the field's value is identically false and `conflicts` is omitted. +`raft_group_ids[]` and `leader_terms[]` are omitted only for legacy +peers or rows whose identity is unknown. ## 6. SPA changes @@ -330,10 +324,9 @@ identically false. §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. +- `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 @@ -341,37 +334,40 @@ 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 +## 7. Implementation status -PR 1 (this design + a small slice): +Implemented server slice: -- Land this design doc. -- Wire `--keyvizFanoutNodes` flag plumbing through `main.go` → +- `--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. +- `KeyVizRow` JSON fields `conflicts`, `raft_group_ids`, and + `leader_terms`, each parallel to `values[]`. +- Fan-out aggregator with the §4 merge rules, degraded peer status, + peer timeouts, bounded peer response bodies, cookie forwarding, and + recursion guard. +- 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. -PR 2: +Implemented SPA slice: -- SPA: degraded banner, conflict hatching, header counter. +- Degraded banner, conflict hatching, and cluster-view header + counter. -PR 3 (Phase 2-C+): +Deferred follow-ups: -- 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. +- 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. 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. + 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 @@ -405,17 +401,19 @@ PR 3 (Phase 2-C+): 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. (Review finding on 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. +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` - 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. + 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 From b1a34d5ebb3063758392c0fdf2892df2c86c55f3 Mon Sep 17 00:00:00 2001 From: bootjp Date: Wed, 8 Jul 2026 18:23:47 +0900 Subject: [PATCH 04/17] docs: align keyviz fanout contract --- docs/admin_ui_key_visualizer_design.md | 12 ++++---- ...04_27_implemented_keyviz_cluster_fanout.md | 30 +++++++------------ ...26_04_28_proposed_keyviz_adapter_labels.md | 15 ++++++---- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index e79a1f787..18e74f736 100644 --- a/docs/admin_ui_key_visualizer_design.md +++ b/docs/admin_ui_key_visualizer_design.md @@ -283,13 +283,13 @@ Phases 0–2 require no Raft or FSM changes. Data-plane protocol adapters only r ### 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 writes are recorded by Raft leaders 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 diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index e910a61df..128d43e90 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -282,11 +282,9 @@ no breaking changes — old SPA versions keep working): // 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": []} + {"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 @@ -294,12 +292,10 @@ no breaking changes — old SPA versions keep working): } ``` -`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. (Round-1 review finding on PR #685.) +`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. 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 @@ -307,14 +303,10 @@ where the fan-out merge saw disagreement inside the same is the row-level OR of `conflicts[]` and remains on the wire for older SPAs that only know how to hatch a whole row. -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` or allocates `conflicts[]`; in local-only mode -the field's value is identically false and `conflicts` is omitted. +`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 or rows whose identity is unknown. diff --git a/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md b/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md index d760e87ff..0c430a617 100644 --- a/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md +++ b/docs/design/2026_04_28_proposed_keyviz_adapter_labels.md @@ -587,12 +587,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`) From b0be2638806aad955200fdd494a41bc59d5a8f63 Mon Sep 17 00:00:00 2001 From: bootjp Date: Wed, 8 Jul 2026 18:41:38 +0900 Subject: [PATCH 05/17] docs: align keyviz fanout wire sample --- .../design/2026_04_27_implemented_keyviz_cluster_fanout.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index 128d43e90..ef72264fc 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -270,7 +270,7 @@ no breaking changes — old SPA versions keep working): "bucket_id": "route:42", ..., "values": [...], - "conflict": false, // OR of conflicts[] + "conflict": true, // OR of conflicts[] "conflicts": [false, true, false], "raft_group_ids": [7, 7, 7], "leader_terms": [42, 43, 43] @@ -308,7 +308,10 @@ 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 or rows whose identity is unknown. +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 From 104eb4f47b2bc16d1e7f7ecc484f0486b1bb3b2b Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 9 Jul 2026 04:25:35 +0900 Subject: [PATCH 06/17] Reserve retired raftadmin transfer fields --- proto/service.pb.go | 4 ++-- proto/service.proto | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/proto/service.pb.go b/proto/service.pb.go index c79e74686..1db8485d0 100644 --- a/proto/service.pb.go +++ b/proto/service.pb.go @@ -2304,10 +2304,10 @@ const file_service_proto_rawDesc = "" + "\x02id\x18\x01 \x01(\tR\x02id\x12%\n" + "\x0eprevious_index\x18\x02 \x01(\x04R\rpreviousIndex\"<\n" + "$RaftAdminConfigurationChangeResponse\x12\x14\n" + - "\x05index\x18\x01 \x01(\x04R\x05index\"h\n" + + "\x05index\x18\x01 \x01(\x04R\x05index\"\x9d\x01\n" + "\"RaftAdminTransferLeadershipRequest\x12\x1b\n" + "\ttarget_id\x18\x01 \x01(\tR\btargetId\x12%\n" + - "\x0etarget_address\x18\x02 \x01(\tR\rtargetAddress\"%\n" + + "\x0etarget_address\x18\x02 \x01(\tR\rtargetAddressJ\x04\b\x03\x10\x04J\x04\b\x04\x10\x05J\x04\b\x05\x10\x06R\x05gatedR\amax_lagR\x11target_candidates\"%\n" + "#RaftAdminTransferLeadershipResponse*\xa9\x01\n" + "\x0eRaftAdminState\x12\x1c\n" + "\x18RAFT_ADMIN_STATE_UNKNOWN\x10\x00\x12\x1d\n" + diff --git a/proto/service.proto b/proto/service.proto index 206b32bb9..2de66265b 100644 --- a/proto/service.proto +++ b/proto/service.proto @@ -248,6 +248,9 @@ message RaftAdminConfigurationChangeResponse { } message RaftAdminTransferLeadershipRequest { + reserved 3, 4, 5; + reserved "gated", "max_lag", "target_candidates"; + string target_id = 1; string target_address = 2; } From ca4f1030452b42c16e247072a59ff7cb665e7f6c Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 9 Jul 2026 04:41:56 +0900 Subject: [PATCH 07/17] Align keyviz fanout docs with shipped merge --- ...04_27_implemented_keyviz_cluster_fanout.md | 43 ++-- proto/service.pb.go | 196 +++++++++++++----- proto/service.proto | 11 +- 3 files changed, 168 insertions(+), 82 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index ef72264fc..da00313bf 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -227,35 +227,31 @@ 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. +divergence the operator should investigate. Phase 2-C does not ship +a structured warning field for this case; the aggregator keeps the +deterministic metadata winner and still serves the merged response. +Surfacing catalog-divergence warnings in the per-node status payload +is a future wire extension. ### 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. +All nodes use `keyvizStep` (default 1 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: -- Both sides round `column_unix_ms[i]` down to the nearest - multiple of `keyvizStep_ms` before merging. +- 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). -- 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. (Round-1 review finding on PR #685.) +- 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 @@ -337,9 +333,10 @@ Implemented server slice: `internal/admin` → handler. - `KeyVizRow` JSON fields `conflicts`, `raft_group_ids`, and `leader_terms`, each parallel to `values[]`. -- Fan-out aggregator with the §4 merge rules, degraded peer status, - peer timeouts, bounded peer response bodies, cookie forwarding, and - recursion guard. +- 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. diff --git a/proto/service.pb.go b/proto/service.pb.go index 1db8485d0..7ea9d64a9 100644 --- a/proto/service.pb.go +++ b/proto/service.pb.go @@ -2087,7 +2087,7 @@ func (x *RaftAdminConfigurationChangeResponse) GetIndex() uint64 { return 0 } -type RaftAdminTransferLeadershipRequest struct { +type TransferTarget struct { state protoimpl.MessageState `protogen:"open.v1"` TargetId string `protobuf:"bytes,1,opt,name=target_id,json=targetId,proto3" json:"target_id,omitempty"` TargetAddress string `protobuf:"bytes,2,opt,name=target_address,json=targetAddress,proto3" json:"target_address,omitempty"` @@ -2095,9 +2095,64 @@ type RaftAdminTransferLeadershipRequest struct { sizeCache protoimpl.SizeCache } +func (x *TransferTarget) Reset() { + *x = TransferTarget{} + mi := &file_service_proto_msgTypes[37] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *TransferTarget) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*TransferTarget) ProtoMessage() {} + +func (x *TransferTarget) ProtoReflect() protoreflect.Message { + mi := &file_service_proto_msgTypes[37] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use TransferTarget.ProtoReflect.Descriptor instead. +func (*TransferTarget) Descriptor() ([]byte, []int) { + return file_service_proto_rawDescGZIP(), []int{37} +} + +func (x *TransferTarget) GetTargetId() string { + if x != nil { + return x.TargetId + } + return "" +} + +func (x *TransferTarget) GetTargetAddress() string { + if x != nil { + return x.TargetAddress + } + return "" +} + +type RaftAdminTransferLeadershipRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + TargetId string `protobuf:"bytes,1,opt,name=target_id,json=targetId,proto3" json:"target_id,omitempty"` + TargetAddress string `protobuf:"bytes,2,opt,name=target_address,json=targetAddress,proto3" json:"target_address,omitempty"` + Gated bool `protobuf:"varint,3,opt,name=gated,proto3" json:"gated,omitempty"` + MaxLag uint64 `protobuf:"varint,4,opt,name=max_lag,json=maxLag,proto3" json:"max_lag,omitempty"` + TargetCandidates []*TransferTarget `protobuf:"bytes,5,rep,name=target_candidates,json=targetCandidates,proto3" json:"target_candidates,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + func (x *RaftAdminTransferLeadershipRequest) Reset() { *x = RaftAdminTransferLeadershipRequest{} - mi := &file_service_proto_msgTypes[37] + mi := &file_service_proto_msgTypes[38] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -2109,7 +2164,7 @@ func (x *RaftAdminTransferLeadershipRequest) String() string { func (*RaftAdminTransferLeadershipRequest) ProtoMessage() {} func (x *RaftAdminTransferLeadershipRequest) ProtoReflect() protoreflect.Message { - mi := &file_service_proto_msgTypes[37] + mi := &file_service_proto_msgTypes[38] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -2122,7 +2177,7 @@ func (x *RaftAdminTransferLeadershipRequest) ProtoReflect() protoreflect.Message // Deprecated: Use RaftAdminTransferLeadershipRequest.ProtoReflect.Descriptor instead. func (*RaftAdminTransferLeadershipRequest) Descriptor() ([]byte, []int) { - return file_service_proto_rawDescGZIP(), []int{37} + return file_service_proto_rawDescGZIP(), []int{38} } func (x *RaftAdminTransferLeadershipRequest) GetTargetId() string { @@ -2139,6 +2194,27 @@ func (x *RaftAdminTransferLeadershipRequest) GetTargetAddress() string { return "" } +func (x *RaftAdminTransferLeadershipRequest) GetGated() bool { + if x != nil { + return x.Gated + } + return false +} + +func (x *RaftAdminTransferLeadershipRequest) GetMaxLag() uint64 { + if x != nil { + return x.MaxLag + } + return 0 +} + +func (x *RaftAdminTransferLeadershipRequest) GetTargetCandidates() []*TransferTarget { + if x != nil { + return x.TargetCandidates + } + return nil +} + type RaftAdminTransferLeadershipResponse struct { state protoimpl.MessageState `protogen:"open.v1"` unknownFields protoimpl.UnknownFields @@ -2147,7 +2223,7 @@ type RaftAdminTransferLeadershipResponse struct { func (x *RaftAdminTransferLeadershipResponse) Reset() { *x = RaftAdminTransferLeadershipResponse{} - mi := &file_service_proto_msgTypes[38] + mi := &file_service_proto_msgTypes[39] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -2159,7 +2235,7 @@ func (x *RaftAdminTransferLeadershipResponse) String() string { func (*RaftAdminTransferLeadershipResponse) ProtoMessage() {} func (x *RaftAdminTransferLeadershipResponse) ProtoReflect() protoreflect.Message { - mi := &file_service_proto_msgTypes[38] + mi := &file_service_proto_msgTypes[39] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -2172,7 +2248,7 @@ func (x *RaftAdminTransferLeadershipResponse) ProtoReflect() protoreflect.Messag // Deprecated: Use RaftAdminTransferLeadershipResponse.ProtoReflect.Descriptor instead. func (*RaftAdminTransferLeadershipResponse) Descriptor() ([]byte, []int) { - return file_service_proto_rawDescGZIP(), []int{38} + return file_service_proto_rawDescGZIP(), []int{39} } var File_service_proto protoreflect.FileDescriptor @@ -2304,10 +2380,16 @@ const file_service_proto_rawDesc = "" + "\x02id\x18\x01 \x01(\tR\x02id\x12%\n" + "\x0eprevious_index\x18\x02 \x01(\x04R\rpreviousIndex\"<\n" + "$RaftAdminConfigurationChangeResponse\x12\x14\n" + - "\x05index\x18\x01 \x01(\x04R\x05index\"\x9d\x01\n" + + "\x05index\x18\x01 \x01(\x04R\x05index\"T\n" + + "\x0eTransferTarget\x12\x1b\n" + + "\ttarget_id\x18\x01 \x01(\tR\btargetId\x12%\n" + + "\x0etarget_address\x18\x02 \x01(\tR\rtargetAddress\"\xd5\x01\n" + "\"RaftAdminTransferLeadershipRequest\x12\x1b\n" + "\ttarget_id\x18\x01 \x01(\tR\btargetId\x12%\n" + - "\x0etarget_address\x18\x02 \x01(\tR\rtargetAddressJ\x04\b\x03\x10\x04J\x04\b\x04\x10\x05J\x04\b\x05\x10\x06R\x05gatedR\amax_lagR\x11target_candidates\"%\n" + + "\x0etarget_address\x18\x02 \x01(\tR\rtargetAddress\x12\x14\n" + + "\x05gated\x18\x03 \x01(\bR\x05gated\x12\x17\n" + + "\amax_lag\x18\x04 \x01(\x04R\x06maxLag\x12<\n" + + "\x11target_candidates\x18\x05 \x03(\v2\x0f.TransferTargetR\x10targetCandidates\"%\n" + "#RaftAdminTransferLeadershipResponse*\xa9\x01\n" + "\x0eRaftAdminState\x12\x1c\n" + "\x18RAFT_ADMIN_STATE_UNKNOWN\x10\x00\x12\x1d\n" + @@ -2352,7 +2434,7 @@ func file_service_proto_rawDescGZIP() []byte { } var file_service_proto_enumTypes = make([]protoimpl.EnumInfo, 1) -var file_service_proto_msgTypes = make([]protoimpl.MessageInfo, 39) +var file_service_proto_msgTypes = make([]protoimpl.MessageInfo, 40) var file_service_proto_goTypes = []any{ (RaftAdminState)(0), // 0: RaftAdminState (*RawPutRequest)(nil), // 1: RawPutRequest @@ -2392,8 +2474,9 @@ var file_service_proto_goTypes = []any{ (*RaftAdminPromoteLearnerRequest)(nil), // 35: RaftAdminPromoteLearnerRequest (*RaftAdminRemoveServerRequest)(nil), // 36: RaftAdminRemoveServerRequest (*RaftAdminConfigurationChangeResponse)(nil), // 37: RaftAdminConfigurationChangeResponse - (*RaftAdminTransferLeadershipRequest)(nil), // 38: RaftAdminTransferLeadershipRequest - (*RaftAdminTransferLeadershipResponse)(nil), // 39: RaftAdminTransferLeadershipResponse + (*TransferTarget)(nil), // 38: TransferTarget + (*RaftAdminTransferLeadershipRequest)(nil), // 39: RaftAdminTransferLeadershipRequest + (*RaftAdminTransferLeadershipResponse)(nil), // 40: RaftAdminTransferLeadershipResponse } var file_service_proto_depIdxs = []int32{ 10, // 0: RawScanAtResponse.kv:type_name -> RawKVPair @@ -2404,49 +2487,50 @@ var file_service_proto_depIdxs = []int32{ 18, // 5: CommitResponse.errors:type_name -> KeyError 0, // 6: RaftAdminStatusResponse.state:type_name -> RaftAdminState 31, // 7: RaftAdminConfigurationResponse.servers:type_name -> RaftAdminMember - 1, // 8: RawKV.RawPut:input_type -> RawPutRequest - 3, // 9: RawKV.RawGet:input_type -> RawGetRequest - 5, // 10: RawKV.RawDelete:input_type -> RawDeleteRequest - 7, // 11: RawKV.RawLatestCommitTS:input_type -> RawLatestCommitTSRequest - 9, // 12: RawKV.RawScanAt:input_type -> RawScanAtRequest - 12, // 13: TransactionalKV.Put:input_type -> PutRequest - 16, // 14: TransactionalKV.Get:input_type -> GetRequest - 14, // 15: TransactionalKV.Delete:input_type -> DeleteRequest - 20, // 16: TransactionalKV.Scan:input_type -> ScanRequest - 22, // 17: TransactionalKV.PreWrite:input_type -> PreWriteRequest - 24, // 18: TransactionalKV.Commit:input_type -> CommitRequest - 26, // 19: TransactionalKV.Rollback:input_type -> RollbackRequest - 28, // 20: RaftAdmin.Status:input_type -> RaftAdminStatusRequest - 30, // 21: RaftAdmin.Configuration:input_type -> RaftAdminConfigurationRequest - 33, // 22: RaftAdmin.AddVoter:input_type -> RaftAdminAddVoterRequest - 34, // 23: RaftAdmin.AddLearner:input_type -> RaftAdminAddLearnerRequest - 35, // 24: RaftAdmin.PromoteLearner:input_type -> RaftAdminPromoteLearnerRequest - 36, // 25: RaftAdmin.RemoveServer:input_type -> RaftAdminRemoveServerRequest - 38, // 26: RaftAdmin.TransferLeadership:input_type -> RaftAdminTransferLeadershipRequest - 2, // 27: RawKV.RawPut:output_type -> RawPutResponse - 4, // 28: RawKV.RawGet:output_type -> RawGetResponse - 6, // 29: RawKV.RawDelete:output_type -> RawDeleteResponse - 8, // 30: RawKV.RawLatestCommitTS:output_type -> RawLatestCommitTSResponse - 11, // 31: RawKV.RawScanAt:output_type -> RawScanAtResponse - 13, // 32: TransactionalKV.Put:output_type -> PutResponse - 17, // 33: TransactionalKV.Get:output_type -> GetResponse - 15, // 34: TransactionalKV.Delete:output_type -> DeleteResponse - 21, // 35: TransactionalKV.Scan:output_type -> ScanResponse - 23, // 36: TransactionalKV.PreWrite:output_type -> PreCommitResponse - 25, // 37: TransactionalKV.Commit:output_type -> CommitResponse - 27, // 38: TransactionalKV.Rollback:output_type -> RollbackResponse - 29, // 39: RaftAdmin.Status:output_type -> RaftAdminStatusResponse - 32, // 40: RaftAdmin.Configuration:output_type -> RaftAdminConfigurationResponse - 37, // 41: RaftAdmin.AddVoter:output_type -> RaftAdminConfigurationChangeResponse - 37, // 42: RaftAdmin.AddLearner:output_type -> RaftAdminConfigurationChangeResponse - 37, // 43: RaftAdmin.PromoteLearner:output_type -> RaftAdminConfigurationChangeResponse - 37, // 44: RaftAdmin.RemoveServer:output_type -> RaftAdminConfigurationChangeResponse - 39, // 45: RaftAdmin.TransferLeadership:output_type -> RaftAdminTransferLeadershipResponse - 27, // [27:46] is the sub-list for method output_type - 8, // [8:27] is the sub-list for method input_type - 8, // [8:8] is the sub-list for extension type_name - 8, // [8:8] is the sub-list for extension extendee - 0, // [0:8] is the sub-list for field type_name + 38, // 8: RaftAdminTransferLeadershipRequest.target_candidates:type_name -> TransferTarget + 1, // 9: RawKV.RawPut:input_type -> RawPutRequest + 3, // 10: RawKV.RawGet:input_type -> RawGetRequest + 5, // 11: RawKV.RawDelete:input_type -> RawDeleteRequest + 7, // 12: RawKV.RawLatestCommitTS:input_type -> RawLatestCommitTSRequest + 9, // 13: RawKV.RawScanAt:input_type -> RawScanAtRequest + 12, // 14: TransactionalKV.Put:input_type -> PutRequest + 16, // 15: TransactionalKV.Get:input_type -> GetRequest + 14, // 16: TransactionalKV.Delete:input_type -> DeleteRequest + 20, // 17: TransactionalKV.Scan:input_type -> ScanRequest + 22, // 18: TransactionalKV.PreWrite:input_type -> PreWriteRequest + 24, // 19: TransactionalKV.Commit:input_type -> CommitRequest + 26, // 20: TransactionalKV.Rollback:input_type -> RollbackRequest + 28, // 21: RaftAdmin.Status:input_type -> RaftAdminStatusRequest + 30, // 22: RaftAdmin.Configuration:input_type -> RaftAdminConfigurationRequest + 33, // 23: RaftAdmin.AddVoter:input_type -> RaftAdminAddVoterRequest + 34, // 24: RaftAdmin.AddLearner:input_type -> RaftAdminAddLearnerRequest + 35, // 25: RaftAdmin.PromoteLearner:input_type -> RaftAdminPromoteLearnerRequest + 36, // 26: RaftAdmin.RemoveServer:input_type -> RaftAdminRemoveServerRequest + 39, // 27: RaftAdmin.TransferLeadership:input_type -> RaftAdminTransferLeadershipRequest + 2, // 28: RawKV.RawPut:output_type -> RawPutResponse + 4, // 29: RawKV.RawGet:output_type -> RawGetResponse + 6, // 30: RawKV.RawDelete:output_type -> RawDeleteResponse + 8, // 31: RawKV.RawLatestCommitTS:output_type -> RawLatestCommitTSResponse + 11, // 32: RawKV.RawScanAt:output_type -> RawScanAtResponse + 13, // 33: TransactionalKV.Put:output_type -> PutResponse + 17, // 34: TransactionalKV.Get:output_type -> GetResponse + 15, // 35: TransactionalKV.Delete:output_type -> DeleteResponse + 21, // 36: TransactionalKV.Scan:output_type -> ScanResponse + 23, // 37: TransactionalKV.PreWrite:output_type -> PreCommitResponse + 25, // 38: TransactionalKV.Commit:output_type -> CommitResponse + 27, // 39: TransactionalKV.Rollback:output_type -> RollbackResponse + 29, // 40: RaftAdmin.Status:output_type -> RaftAdminStatusResponse + 32, // 41: RaftAdmin.Configuration:output_type -> RaftAdminConfigurationResponse + 37, // 42: RaftAdmin.AddVoter:output_type -> RaftAdminConfigurationChangeResponse + 37, // 43: RaftAdmin.AddLearner:output_type -> RaftAdminConfigurationChangeResponse + 37, // 44: RaftAdmin.PromoteLearner:output_type -> RaftAdminConfigurationChangeResponse + 37, // 45: RaftAdmin.RemoveServer:output_type -> RaftAdminConfigurationChangeResponse + 40, // 46: RaftAdmin.TransferLeadership:output_type -> RaftAdminTransferLeadershipResponse + 28, // [28:47] is the sub-list for method output_type + 9, // [9:28] is the sub-list for method input_type + 9, // [9:9] is the sub-list for extension type_name + 9, // [9:9] is the sub-list for extension extendee + 0, // [0:9] is the sub-list for field type_name } func init() { file_service_proto_init() } @@ -2460,7 +2544,7 @@ func file_service_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_service_proto_rawDesc), len(file_service_proto_rawDesc)), NumEnums: 1, - NumMessages: 39, + NumMessages: 40, NumExtensions: 0, NumServices: 3, }, diff --git a/proto/service.proto b/proto/service.proto index 2de66265b..b639faf5a 100644 --- a/proto/service.proto +++ b/proto/service.proto @@ -247,12 +247,17 @@ message RaftAdminConfigurationChangeResponse { uint64 index = 1; } -message RaftAdminTransferLeadershipRequest { - reserved 3, 4, 5; - reserved "gated", "max_lag", "target_candidates"; +message TransferTarget { + string target_id = 1; + string target_address = 2; +} +message RaftAdminTransferLeadershipRequest { string target_id = 1; string target_address = 2; + bool gated = 3; + uint64 max_lag = 4; + repeated TransferTarget target_candidates = 5; } message RaftAdminTransferLeadershipResponse {} From 22f2dc5e8aa34f0e32bd81f06ba61a0e7d8b911c Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 9 Jul 2026 04:59:16 +0900 Subject: [PATCH 08/17] Reject unsupported gated transfer requests --- internal/raftadmin/server.go | 28 +++++++++++++++++++++++----- internal/raftadmin/server_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/internal/raftadmin/server.go b/internal/raftadmin/server.go index 563a60f53..d1c9c5bba 100644 --- a/internal/raftadmin/server.go +++ b/internal/raftadmin/server.go @@ -156,11 +156,15 @@ func (s *Server) TransferLeadership(ctx context.Context, req *pb.RaftAdminTransf if s == nil || s.admin == nil { return nil, grpcStatus(codes.Unimplemented, "leadership transfer is not supported by this raft engine") } - if req != nil && (req.TargetId != "" || req.TargetAddress != "") { - if req.TargetId == "" || req.TargetAddress == "" { - return nil, grpcStatus(codes.InvalidArgument, "target_id and target_address must be provided together") - } - if err := s.admin.TransferLeadershipToServer(ctx, req.TargetId, req.TargetAddress); err != nil { + if req != nil && usesUnsupportedGatedTransfer(req) { + return nil, grpcStatus(codes.FailedPrecondition, "gated leadership transfer is not supported by this raft engine") + } + targetID, targetAddress, hasTarget, err := transferLeadershipTarget(req) + if err != nil { + return nil, err + } + if hasTarget { + if err := s.admin.TransferLeadershipToServer(ctx, targetID, targetAddress); err != nil { return nil, adminError(err) } return &pb.RaftAdminTransferLeadershipResponse{}, nil @@ -171,6 +175,20 @@ func (s *Server) TransferLeadership(ctx context.Context, req *pb.RaftAdminTransf return &pb.RaftAdminTransferLeadershipResponse{}, nil } +func transferLeadershipTarget(req *pb.RaftAdminTransferLeadershipRequest) (string, string, bool, error) { + if req == nil || (req.GetTargetId() == "" && req.GetTargetAddress() == "") { + return "", "", false, nil + } + if req.GetTargetId() == "" || req.GetTargetAddress() == "" { + return "", "", false, grpcStatus(codes.InvalidArgument, "target_id and target_address must be provided together") + } + return req.GetTargetId(), req.GetTargetAddress(), true, nil +} + +func usesUnsupportedGatedTransfer(req *pb.RaftAdminTransferLeadershipRequest) bool { + return req.GetGated() || req.GetMaxLag() != 0 || len(req.GetTargetCandidates()) != 0 +} + func stateToProto(state raftengine.State) pb.RaftAdminState { switch state { case raftengine.StateFollower: diff --git a/internal/raftadmin/server_test.go b/internal/raftadmin/server_test.go index 9d0326b3f..b427eb5d4 100644 --- a/internal/raftadmin/server_test.go +++ b/internal/raftadmin/server_test.go @@ -11,8 +11,10 @@ import ( pb "github.com/bootjp/elastickv/proto" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" healthpb "google.golang.org/grpc/health/grpc_health_v1" + "google.golang.org/grpc/status" "google.golang.org/grpc/test/bufconn" ) @@ -262,6 +264,28 @@ func TestServerMapsEngineAdminMethods(t *testing.T) { require.Equal(t, []fakeTransferCall{{id: "node-2", address: "127.0.0.1:50052"}}, engine.targetTransferCalls) } +func TestTransferLeadershipRejectsUnsupportedGatedRequest(t *testing.T) { + t.Parallel() + + engine := &fakeEngine{} + server := NewServer(engine) + + _, err := server.TransferLeadership(context.Background(), &pb.RaftAdminTransferLeadershipRequest{ + Gated: true, + TargetCandidates: []*pb.TransferTarget{{ + TargetId: "node-2", + TargetAddress: "127.0.0.1:50052", + }}, + }) + require.Error(t, err) + require.Equal(t, codes.FailedPrecondition, status.Code(err)) + + engine.mu.Lock() + defer engine.mu.Unlock() + require.Zero(t, engine.transferCalls) + require.Empty(t, engine.targetTransferCalls) +} + func TestRegisterOperationalServicesPublishesLeaderHealth(t *testing.T) { t.Parallel() From e03fb75921f289cacc2e1657ea0eb9b37bc45e46 Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 9 Jul 2026 05:17:01 +0900 Subject: [PATCH 09/17] Document local-first keyviz metadata merge --- ...04_27_implemented_keyviz_cluster_fanout.md | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index da00313bf..bc5eb6c09 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -217,19 +217,19 @@ unknown-identity max fallback). ### 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` +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 the operator should investigate. Phase 2-C does not ship a structured warning field for this case; the aggregator keeps the -deterministic metadata winner and still serves the merged response. +local-first metadata winner and still serves the merged response. Surfacing catalog-divergence warnings in the per-node status payload is a future wire extension. From 8235b386cd5f9f5fc4001a1eb89ea5ca0e7f2826 Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 9 Jul 2026 05:23:38 +0900 Subject: [PATCH 10/17] Clarify keyviz catalog divergence status --- ...026_04_27_implemented_keyviz_cluster_fanout.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index bc5eb6c09..2ea74e479 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -227,11 +227,11 @@ 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 the operator should investigate. Phase 2-C does not ship -a structured warning field for this case; the aggregator keeps the -local-first metadata winner and still serves the merged response. -Surfacing catalog-divergence warnings in the per-node status payload -is a future wire extension. +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 @@ -290,8 +290,9 @@ no breaking changes — old SPA versions keep working): `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. A -future warning array would be an additive wire extension. +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 From 1dbadac3e7c29efc25c5a289b0ade0b6a05109b1 Mon Sep 17 00:00:00 2001 From: bootjp Date: Fri, 10 Jul 2026 23:23:56 +0900 Subject: [PATCH 11/17] docs: align keyviz fanout rollout contract --- ...6_04_27_implemented_keyviz_cluster_fanout.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index 2ea74e479..84634772f 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -33,7 +33,9 @@ rollout are correct by construction: 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. + 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 @@ -93,23 +95,23 @@ identity needed for the canonical §9.1 write merge. ```sh elastickv \ --address 127.0.0.1:50051 \ - --adminAddress 127.0.0.1:8080 \ + --adminListen 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 + `--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 `--adminAddress` is the primary + - **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 `--adminAddress=0.0.0.0:8080` but stamp + 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 @@ -117,7 +119,7 @@ elastickv \ 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. + `--adminListen` 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 @@ -324,7 +326,8 @@ represented by zero entries, matching `proto/admin.proto`'s 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). +renders the merged values (it just ignores the new fields and lacks +the degraded/conflict affordances). ## 7. Implementation status From c55050113c762f05ac9d02c947f33b8e8beceea5 Mon Sep 17 00:00:00 2001 From: bootjp Date: Fri, 10 Jul 2026 23:59:54 +0900 Subject: [PATCH 12/17] docs: clarify keyviz fanout rollout semantics --- ...04_27_implemented_keyviz_cluster_fanout.md | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index 84634772f..b43bfbf10 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -65,7 +65,8 @@ identity needed for the canonical §9.1 write merge. `(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)`. + 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 @@ -94,12 +95,21 @@ identity needed for the canonical §9.1 write merge. ```sh elastickv \ - --address 127.0.0.1:50051 \ - --adminListen 127.0.0.1:8080 \ + --address 10.0.0.1:50051 \ + --adminEnabled \ + --adminListen 10.0.0.1:8080 \ + --adminAllowPlaintextNonLoopback \ --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. Non-loopback plaintext requires +the explicit `--adminAllowPlaintextNonLoopback` opt-in shown above. + - 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 @@ -121,11 +131,12 @@ elastickv \ per request) but wasteful; operators should prefer the literal `--adminListen` 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. +- 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`. + The URL builder preserves explicit schemes, so TLS fan-out does not + require a separate flag. - **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 @@ -149,8 +160,8 @@ elastickv \ 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 + (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. @@ -201,14 +212,18 @@ columns or even in the same column. The merge: - 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` only when two sources report different - non-zero values for the same `(group, term, column)` identity. +- sets `conflicts[j]=true` when two sources report different non-zero + values for the same `(group, term, column)` identity. 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. +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 @@ -298,9 +313,10 @@ 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. `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. +`(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 From 3eee0232c6a826a177ae73e87713bd8efa3f8814 Mon Sep 17 00:00:00 2001 From: bootjp Date: Sat, 11 Jul 2026 00:19:16 +0900 Subject: [PATCH 13/17] docs: clarify keyviz fanout rollout requirements --- ...04_27_implemented_keyviz_cluster_fanout.md | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index b43bfbf10..bfa1c54a2 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -98,7 +98,10 @@ 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 \ --adminAllowPlaintextNonLoopback \ + --adminAllowInsecureDevCookie \ --keyvizEnabled \ --keyvizFanoutNodes=10.0.0.1:8080,10.0.0.2:8080,10.0.0.3:8080 ``` @@ -107,8 +110,17 @@ 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. Non-loopback plaintext requires -the explicit `--adminAllowPlaintextNonLoopback` opt-in shown above. +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. 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 @@ -127,16 +139,23 @@ the explicit `--adminAllowPlaintextNonLoopback` opt-in shown above. - **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 - `--adminListen` value in the flag. + 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`. The URL builder preserves explicit schemes, so TLS fan-out does not - require a separate flag. + 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 From cc4e448c3ee6ddbd506147a8f2d5f5e2f7ed86b1 Mon Sep 17 00:00:00 2001 From: bootjp Date: Sat, 11 Jul 2026 00:34:05 +0900 Subject: [PATCH 14/17] docs: align keyviz fanout operations guide --- docs/admin_ui_key_visualizer_design.md | 28 +++++++++++++++---- ...04_27_implemented_keyviz_cluster_fanout.md | 24 ++++++++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index 115b13959..b50379d90 100644 --- a/docs/admin_ui_key_visualizer_design.md +++ b/docs/admin_ui_key_visualizer_design.md @@ -76,7 +76,10 @@ The admin binary holds no authoritative state. All data is fetched on demand fro - 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. +- For the original 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 @@ -276,9 +279,21 @@ 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 @@ -298,7 +313,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 diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index bfa1c54a2..aeaf58be3 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -100,6 +100,7 @@ elastickv \ --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 \ @@ -114,7 +115,11 @@ 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. Non-loopback plaintext requires the explicit +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 @@ -149,13 +154,16 @@ rollouts should use admin TLS and omit both plaintext escape hatches. 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`. - 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. + 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 From cffe865b7a0217993f2944577140c1bbb4f99821 Mon Sep 17 00:00:00 2001 From: bootjp Date: Sat, 11 Jul 2026 00:38:52 +0900 Subject: [PATCH 15/17] docs: separate keyviz fanout architecture paths --- docs/admin_ui_key_visualizer_design.md | 28 +++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index b50379d90..49807cce2 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 + WebSocket" --> 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,7 +73,13 @@ 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). +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 Why a separate binary From e51f140a5bd79eb079e893fc1d44c4d8807cd9dc Mon Sep 17 00:00:00 2001 From: bootjp Date: Sat, 11 Jul 2026 00:57:59 +0900 Subject: [PATCH 16/17] docs: align keyviz fanout shipped contract --- docs/admin_ui_key_visualizer_design.md | 25 ++++++++++------ ...04_27_implemented_keyviz_cluster_fanout.md | 29 ++++++++++++------- scripts/rolling-update.env.example | 13 +++++---- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index 49807cce2..6536da04e 100644 --- a/docs/admin_ui_key_visualizer_design.md +++ b/docs/admin_ui_key_visualizer_design.md @@ -51,7 +51,7 @@ flowchart LR Node3["Node C"] end - Browser -- "HTTP/JSON + WebSocket" --> Node1HTTP + Browser -- "HTTP/JSON polling" --> Node1HTTP Node1HTTP -- "admin HTTP peer fan-out" --> Node2HTTP Node1HTTP -- "admin HTTP peer fan-out" --> Node3HTTP @@ -81,12 +81,19 @@ 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 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. -- For the original standalone overview path, +### 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. @@ -334,7 +341,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. @@ -349,7 +356,7 @@ 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. | — | diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index aeaf58be3..5f0c1a1e4 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -222,25 +222,34 @@ not also served by node B. Summation is exact. ### 4.2 Writes → per-cell group/term dedupe -Under stable leadership, exactly one node (the current leader for -the route's group) records writes for a given -`(bucketID, raftGroupID, leaderTerm, column)`; other nodes report -0. Zero-valued contributions are ignored for write conflict -purposes, so follower zeros against the leader's non-zero value do -not perturb the result or raise `conflicts[j]`. +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, because Raft has at most one leader per group - and term; + 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. + 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 @@ -279,7 +288,7 @@ warnings is a future wire extension. ### 4.5 Time alignment -All nodes use `keyvizStep` (default 1 s), but the shipped Phase 2-C +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 diff --git a/scripts/rolling-update.env.example b/scripts/rolling-update.env.example index f71cfee29..ed5e63f12 100644 --- a/scripts/rolling-update.env.example +++ b/scripts/rolling-update.env.example @@ -116,12 +116,13 @@ 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. +# (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" From 17f591765502b3f919bcbd2098cad197f512a65a Mon Sep 17 00:00:00 2001 From: bootjp Date: Sat, 11 Jul 2026 01:22:52 +0900 Subject: [PATCH 17/17] docs: align keyviz fanout API contract --- docs/admin_ui_key_visualizer_design.md | 44 +++++++++++++------ ...04_27_implemented_keyviz_cluster_fanout.md | 20 ++++++--- .../2026_04_29_proposed_logical_backup.md | 7 +-- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/docs/admin_ui_key_visualizer_design.md b/docs/admin_ui_key_visualizer_design.md index 6536da04e..119c2c35d 100644 --- a/docs/admin_ui_key_visualizer_design.md +++ b/docs/admin_ui_key_visualizer_design.md @@ -102,7 +102,7 @@ after restart once Phase 3 persistence is enabled (see §5.6). 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 | |---|---| @@ -110,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. @@ -160,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. @@ -315,7 +327,11 @@ Phases 0–2 require no Raft or FSM changes. Data-plane protocol adapters only r ### 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), 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: +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: - `--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. diff --git a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md index 5f0c1a1e4..02614cb65 100644 --- a/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md +++ b/docs/design/2026_04_27_implemented_keyviz_cluster_fanout.md @@ -442,13 +442,19 @@ Deferred follow-ups: `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. (Review finding on PR #685.) + 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 diff --git a/docs/design/2026_04_29_proposed_logical_backup.md b/docs/design/2026_04_29_proposed_logical_backup.md index 39350d188..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_implemented_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