docs: record ADR 0009 (etcd hot path) and reconciler-sharding spec#930
Merged
Conversation
ADR 0009 settles whether Kubernetes is the wrong substrate for sandboxes against the Mitos claim path. Warm pools already remove kube-scheduler and the readiness watch already removes create latency, so the residual is the per-sandbox etcd write throughput. The decision: keep the Sandbox CR as the durable record, measure the control-plane ceiling (issue #15 item 2) before decoupling, and if it binds, shard the reconciler (Option A) before ever moving the claim into the gateway (Option B, which needs its own ADR and a named security review). Adds the ready-to-execute Option A design spec and indexes the ADR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
📝 WalkthroughWalkthroughAdds ADR 0009 and a reconciler-sharding specification. The documents retain the Sandbox CR as the durable lifecycle record, define claim-safety requirements, specify static hash-based sharding, and gate rollout on control-plane throughput measurements. ChangesSandbox lifecycle scaling
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md`:
- Around line 73-77: Update the static shard membership specification for
--shard-count and --shard-index to require startup validation: shard-count must
be at least 1, and shard-index must be within [0, shard-count). Define invalid
configurations as startup failures before sharding or reconciliation begins,
while preserving the existing defaults and shardCount=1 behavior.
- Around line 109-112: Update the pathological two-owner overlap test design to
configure reconcilers with distinct old and new shard-count/shard-index
assignments that represent a resize remap, rather than giving both the same
index. Extend assertions to cover Sandbox side effects: activation occurs at
most once, token Secret writes are not duplicated, and stale status/finalizer
updates are rejected or ignored while preserving the existing optimistic-lock,
guarded-release, and clean claim-resolution checks.
- Around line 27-43: Define fencing for the entire Sandbox reconcile, including
activation, token Secret, status, and finalizer writes, using ownership epochs
or an equivalent resize-safe protocol before declaring overlap harmless. Update
docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md lines 27-43 to
specify this complete hardening; align
docs/adr/0009-sandbox-lifecycle-state-and-etcd-hot-path.md lines 88-105 with the
selected resize protocol and exclusive-owner invariant; and update
docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md lines 109-112 to
test duplicate activation and Sandbox writes with genuinely different old and
new shard assignments.
- Around line 117-124: Update the “measurement signal that opens the gate”
section to define reproducible metrics for arrival rate, achieved claims/sec,
reconcile work-queue depth, leader CPU saturation, etcd blocking/fsync latency,
and forkd behavior. Specify each metric’s data source, sampling window, and
pass/fail threshold, including how plateauing claims, queue growth, CPU-bound
leadership, and the etcd-throughput alternative are determined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c650bd5a-7779-4068-a696-72b788005449
📒 Files selected for processing (3)
docs/adr/0009-sandbox-lifecycle-state-and-etcd-hot-path.mddocs/adr/README.mddocs/superpowers/specs/2026-07-18-reconciler-sharding-design.md
Comment on lines
+27
to
+43
| ## Key insight: sharding is load distribution, not correctness | ||
|
|
||
| Correctness does NOT have to rest on perfect single-ownership of a Sandbox. The | ||
| pod-claim commit is already the true arbiter: | ||
|
|
||
| - `markHuskPodClaimed` (`huskpod.go`) patches the `mitos.run/claim` label under | ||
| `client.MergeFromWithOptimisticLock`. Two actors that select the same dormant | ||
| pod both attempt the patch; the API server lets exactly one win and the other | ||
| gets 409, which the caller handles by pending + requeue (`HuskPodRaced`). This | ||
| holds no matter how many reconcilers race. | ||
|
|
||
| So if the RELEASE path and the token/status writes are hardened against a second | ||
| writer, then sharding becomes a pure LOAD-DISTRIBUTION optimization: a brief | ||
| two-owner window (for example during a shard-count resize) is merely wasted work | ||
| (a 409 race), never corruption. This is what makes Option A cheap and safe, and | ||
| it is why we do NOT need a perfect exactly-one-owner lease as a correctness | ||
| mechanism. The single hardening prerequisite: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Define fencing for the entire Sandbox reconcile, not only the pod claim label. The documents currently allow resize overlap based on the optimistic pod-label lock, but activation, token Secret, status, and finalizer operations remain single-writer side effects.
docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md#L27-L43: specify ownership fencing/epochs and complete token/status hardening before declaring overlap harmless.docs/adr/0009-sandbox-lifecycle-state-and-etcd-hot-path.md#L88-L105: align Option A’s exclusive-owner invariant with the chosen resize protocol.docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md#L109-L112: test duplicate activation and Sandbox writes using genuinely different old/new shard assignments.
🧰 Tools
🪛 LanguageTool
[style] ~33-~33: Consider using “who” when you are referring to people instead of objects.
Context: ...ergeFromWithOptimisticLock`. Two actors that select the same dormant pod both atte...
(THAT_WHO)
📍 Affects 2 files
docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md#L27-L43(this comment)docs/adr/0009-sandbox-lifecycle-state-and-etcd-hot-path.md#L88-L105docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md#L109-L112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md` around lines
27 - 43, Define fencing for the entire Sandbox reconcile, including activation,
token Secret, status, and finalizer writes, using ownership epochs or an
equivalent resize-safe protocol before declaring overlap harmless. Update
docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md lines 27-43 to
specify this complete hardening; align
docs/adr/0009-sandbox-lifecycle-state-and-etcd-hot-path.md lines 88-105 with the
selected resize protocol and exclusive-owner invariant; and update
docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md lines 109-112 to
test duplicate activation and Sandbox writes with genuinely different old and
new shard assignments.
Comment on lines
+73
to
+77
| - `shard(key) = fnv32a(namespace + "/" + name) % shardCount`. | ||
| - Membership is STATIC in v1: `--shard-count N` (default 1) and `--shard-index i` | ||
| (default 0), where `i` comes from the pod's StatefulSet ordinal via the downward | ||
| API. `shardCount = 1` reproduces today's behavior exactly (one owner of | ||
| everything), so the flag defaults to a no-op. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail closed on invalid shard configuration.
Specify startup validation for shard-count >= 1 and 0 <= shard-index < shard-count. Otherwise shard-count=0 can lead to modulo-by-zero, while an out-of-range index silently filters every object for that replica and can leave part of the workload unreconciled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md` around lines
73 - 77, Update the static shard membership specification for --shard-count and
--shard-index to require startup validation: shard-count must be at least 1, and
shard-index must be within [0, shard-count). Define invalid configurations as
startup failures before sharding or reconciliation begins, while preserving the
existing defaults and shardCount=1 behavior.
Comment on lines
+109
to
+112
| - Pathological two-owner (correctness backstop): two reconcilers BOTH index 0 | ||
| (simulating a resize overlap) claiming from one warm pool. Assert no pod carries | ||
| two claims (the optimistic lock holds), no warm capacity leaks (guarded | ||
| release), and every claim resolves to Ready or a clean pend, never corruption. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the overlap test cover Sandbox side effects and real resize assignments.
The proposed backstop checks pod labels and eventual claim outcomes, but not duplicate activation, token Secret writes, or stale status/finalizer updates. Also configure the two reconcilers with old and new (shard-count, shard-index) assignments; two reconcilers with the same index alone does not model a resize remap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md` around lines
109 - 112, Update the pathological two-owner overlap test design to configure
reconcilers with distinct old and new shard-count/shard-index assignments that
represent a resize remap, rather than giving both the same index. Extend
assertions to cover Sandbox side effects: activation occurs at most once, token
Secret writes are not duplicated, and stale status/finalizer updates are
rejected or ignored while preserving the existing optimistic-lock,
guarded-release, and clean claim-resolution checks.
Comment on lines
+117
to
+124
| ## The measurement signal that opens the gate | ||
|
|
||
| Flip sharding on only when the #15 item 2 data shows ALL of: achieved claims/sec | ||
| plateaus below the arrival rate; the active leader's reconcile work-queue depth | ||
| grows without draining; and the leader pod is CPU-bound (not blocked on etcd or | ||
| forkd). If instead the wall is raw etcd write throughput (leader CPU idle, etcd | ||
| fsync latency climbing), sharding the reconciler will NOT help and the decision | ||
| returns to ADR 0009 Option C or etcd-side work; record that outcome as a new ADR. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial
Make the measurement gate reproducible.
The benchmark configuration defines arrival rate, duration, concurrency, and achieved claims/sec, but this gate additionally requires queue depth, leader CPU saturation, etcd blocking, and forkd behavior. Define the metric names, sampling window, thresholds, and data source so operators cannot reach opposite decisions from the same run.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md` around lines
117 - 124, Update the “measurement signal that opens the gate” section to define
reproducible metrics for arrival rate, achieved claims/sec, reconcile work-queue
depth, leader CPU saturation, etcd blocking/fsync latency, and forkd behavior.
Specify each metric’s data source, sampling window, and pass/fail threshold,
including how plateauing claims, queue growth, CPU-bound leadership, and the
etcd-throughput alternative are determined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
Linked Issues or Issue Description
Related: #15
What Changed
docs/adr/0009-sandbox-lifecycle-state-and-etcd-hot-path.md: the decision that the Sandbox CR stays the durable record, decoupling is gated on the measured control-plane ceiling, and reconciler sharding (Option A) is preferred over moving the claim into the gateway (Option B).docs/superpowers/specs/2026-07-18-reconciler-sharding-design.md: the ready-to-execute Option A spec (hash-in-predicate load distribution, the optimistic-locked claim as the correctness arbiter, the envtest plan, and the measurement signal that opens the gate).docs/adr/README.md.Verification
docs/adr/README.mdformat (Status, Issue, Context, Decision, Consequences) and is indexed.grep -nP '[\x{2013}\x{2014}]'over the three files is clean.Risks
Low risk. Documentation and a proposed ADR only; no runtime, CI, or security surface changes. The ADR is
proposed, notaccepted, and explicitly moves no code.Model Used
Claude Opus 4.8 (1M context), model id
claude-opus-4-8, high reasoning effort.Checklist
Summary by CodeRabbit