Skip to content

Commit 71a8241

Browse files
bootjpbootjp
committed
docs(design): propose Redis one-phase txn dedup default-on flip
Closes the rollout of the option-2 idempotency design landed by docs/design/2026_05_21_proposed_txn_secondary_idempotency.md by flipping adapter.RedisServer.onePhaseTxnDedup from default-off to default-on. No new mechanism — the FSM probe, the PrevCommitTS wire field, and every adapter-side write-set reuse path already ship. Default-on is authorized by the parent design's M4 7-day green criterion in the dedup-mode Jepsen workflow; 12 consecutive green runs on the stress profile (2026-05-31 → 2026-06-10) exceed that threshold. Triggered by Jepsen scheduled-stress run 27033231956 (2026-06-05) on the control baseline workflow (gate off) which surfaced :duplicate-elements and the related real-time cycle anomalies, exactly the shape the parent design was written to prevent. Issue #937 tracks that failure; this proposal closes it by retiring the unprotected default. Doc commit only. Implementation commit follows in the same PR.
1 parent ed82233 commit 71a8241

1 file changed

Lines changed: 217 additions & 0 deletions

File tree

‎docs/design/2026_06_10_proposed_redis_onephase_dedup_default_on.md‎

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
# Redis one-phase txn dedup — flip default to on
2+
3+
Status: Proposed
4+
Author: bootjp
5+
Date: 2026-06-10
6+
7+
> **Status: proposed.** Flip
8+
> `adapter.RedisServer.onePhaseTxnDedup` from default-off to default-on,
9+
> closing the rollout of the option-2 idempotency design landed by
10+
> [`2026_05_21_proposed_txn_secondary_idempotency.md`][parent]. No new
11+
> mechanism — the FSM probe (`dedupProbeOnePhase` in `kv/fsm.go`), the
12+
> wire change (`TxnMeta.PrevCommitTS`, V2-only when non-zero), and every
13+
> adapter-side reuse path (RPUSH/LPUSH, MULTI/EXEC, standalone SET, etc.)
14+
> already ship and have been exercised by the dedup-mode Jepsen workflow
15+
> for **12 consecutive green runs (2026-05-31 → 2026-06-10)**, exceeding
16+
> the parent design's `M4` 7-day criterion. The rolling-upgrade reader
17+
> (probe code) has been on every node in production for months; only the
18+
> writer (emission default) remains.
19+
>
20+
> Triggered by Jepsen Scheduled Stress run
21+
> [27033231956](https://github.com/bootjp/elastickv/actions/runs/27033231956/job/79790587770)
22+
> on 2026-06-05 — the **dedup-OFF control baseline workflow**
23+
> ([`jepsen-test-scheduled.yml`](../../.github/workflows/jepsen-test-scheduled.yml))
24+
> surfaced `:duplicate-elements`, `:future-read`, `:G-single-item-realtime`,
25+
> and `:G2-item-realtime` on the Redis list-append workload, exactly the
26+
> anomaly shape the parent design was written to prevent. Issue
27+
> [#937](https://github.com/bootjp/elastickv/issues/937) tracks that
28+
> failure; this proposal closes it by retiring the unprotected default.
29+
30+
[parent]: 2026_05_21_proposed_txn_secondary_idempotency.md
31+
32+
## Background
33+
34+
The parent design landed an FSM-side dedup probe and an adapter-side
35+
write-set reuse path keyed on a stale `commit_ts` ridden through
36+
`OperationGroup.PrevCommitTS` into a V2 `TxnMeta`. The probe is always
37+
present; emission of `prev_commit_ts != 0` is gated by
38+
`RedisServer.onePhaseTxnDedup` (constructor: `WithOnePhaseTxnDedup`, env:
39+
`ELASTICKV_REDIS_ONEPHASE_DEDUP=1`). The gate stays default-off until
40+
the cluster has uniformly upgraded — the parent's R5 "ship the reader
41+
before the writer" sequencing.
42+
43+
Two Jepsen workflows run the same stress profile (`--time-limit 150
44+
--rate 10 --concurrency 8 --key-count 16 --max-writes-per-key 250
45+
--max-txn-length 4`) every day against `main`:
46+
47+
| Workflow | Env | Purpose |
48+
|---|---|---|
49+
| [`jepsen-test-scheduled.yml`][off] | `ELASTICKV_REDIS_ONEPHASE_DEDUP` unset (off) | Legacy-path baseline — expected to surface the parent's anomaly class until default-on lands. |
50+
| [`jepsen-test-scheduled-dedup.yml`][on] | `ELASTICKV_REDIS_ONEPHASE_DEDUP=1` (on) | M4 validation — must stay green to authorize default-on. |
51+
52+
[off]: ../../.github/workflows/jepsen-test-scheduled.yml
53+
[on]: ../../.github/workflows/jepsen-test-scheduled-dedup.yml
54+
55+
## M4 evidence
56+
57+
The parent design's `M4` criterion is *"7 consecutive days without
58+
`:duplicate-elements` / `:G-single-item-realtime` in the dedup-mode
59+
workflow."*
60+
61+
Dedup-mode (`jepsen-test-scheduled-dedup.yml`) run history on `main`:
62+
63+
| Date (UTC) | Run | Conclusion |
64+
|---|---|---|
65+
| 2026-06-10 04:50 | [27253991270](https://github.com/bootjp/elastickv/actions/runs/27253991270) | success |
66+
| 2026-06-09 04:44 | [27184402445](https://github.com/bootjp/elastickv/actions/runs/27184402445) | success |
67+
| 2026-06-08 04:57 | [27116904871](https://github.com/bootjp/elastickv/actions/runs/27116904871) | success |
68+
| 2026-06-07 04:54 | [27083142341](https://github.com/bootjp/elastickv/actions/runs/27083142341) | success |
69+
| 2026-06-06 04:40 | [27052820868](https://github.com/bootjp/elastickv/actions/runs/27052820868) | success |
70+
| 2026-06-05 04:49 | [26996058409](https://github.com/bootjp/elastickv/actions/runs/26996058409) | success |
71+
| 2026-06-04 04:57 | [26931749014](https://github.com/bootjp/elastickv/actions/runs/26931749014) | success |
72+
| 2026-06-04 04:16 | [26930308744](https://github.com/bootjp/elastickv/actions/runs/26930308744) | success |
73+
| 2026-06-03 04:58 | [26864667493](https://github.com/bootjp/elastickv/actions/runs/26864667493) | success |
74+
| 2026-06-02 04:56 | [26799333263](https://github.com/bootjp/elastickv/actions/runs/26799333263) | success |
75+
| 2026-06-01 04:58 | [26736041841](https://github.com/bootjp/elastickv/actions/runs/26736041841) | success |
76+
| 2026-05-31 04:52 | [26703624971](https://github.com/bootjp/elastickv/actions/runs/26703624971) | success |
77+
78+
12 consecutive green runs over 10 calendar days, on the exact stress
79+
profile that produced the parent design's trigger anomaly. M4 is met.
80+
81+
The control workflow (`jepsen-test-scheduled.yml`, gate off) continues
82+
to surface `:duplicate-elements` and the related real-time cycle
83+
anomalies as expected; the 2026-06-05 18:37 failure ([#937]) is the
84+
most recent observation. The control's purpose ends when default-on
85+
lands — see *§Control workflow disposition* below.
86+
87+
## Proposal
88+
89+
### M1 — Flip the default
90+
91+
`adapter/redis.go` constructs the server with:
92+
93+
```go
94+
onePhaseTxnDedup: os.Getenv("ELASTICKV_REDIS_ONEPHASE_DEDUP") == "1",
95+
```
96+
97+
After this PR:
98+
99+
```go
100+
// Default on; set ELASTICKV_REDIS_ONEPHASE_DEDUP=0 to opt out.
101+
onePhaseTxnDedup: os.Getenv("ELASTICKV_REDIS_ONEPHASE_DEDUP") != "0",
102+
```
103+
104+
The env-var sense inverts from opt-in to opt-out. `=1` (the dedup-mode
105+
workflow's existing setting) and unset both resolve to `true`; only the
106+
explicit `=0` opts out. `WithOnePhaseTxnDedup(bool)` continues to be the
107+
constructor-level override and trumps the env var either way.
108+
109+
Comment and `WithOnePhaseTxnDedup`'s godoc are updated to describe the
110+
new default; the in-file pointer to R5 stays (now reframed as "the
111+
ordering constraint that authorized this default flip").
112+
113+
### M2 — Control workflow disposition
114+
115+
After default-on, `jepsen-test-scheduled.yml` would silently exercise
116+
the same path as `jepsen-test-scheduled-dedup.yml` (unset env → true),
117+
so the two workflows would collapse to the same coverage. Two options:
118+
119+
| Option | Effect | Recommendation |
120+
|---|---|---|
121+
| **A** Keep the control by setting `ELASTICKV_REDIS_ONEPHASE_DEDUP=0` in `jepsen-test-scheduled.yml`'s job env. | Continues to publish the unprotected-path baseline; lets us notice if anomalies on the off-path ever vanish (suggesting the workload regressed in coverage). | Yes for the first 30 days post-flip. After that, retire if signal stays unchanged. |
122+
| **B** Delete `jepsen-test-scheduled.yml` entirely. | One fewer cron consuming runners. Loses the legacy-path baseline. | No, until 30 days of post-flip data shows the dedup-mode workflow is the only signal we read. |
123+
124+
This PR picks **A**: add explicit `ELASTICKV_REDIS_ONEPHASE_DEDUP: "0"`
125+
to `jepsen-test-scheduled.yml`'s top-level `env:` so the control retains
126+
its meaning across the default flip. The 30-day retirement decision
127+
becomes a follow-up issue.
128+
129+
### M3 — Issue #937 closure
130+
131+
Update [#937](https://github.com/bootjp/elastickv/issues/937) with the
132+
M4 evidence + this proposal link, then close as "expected baseline; the
133+
dedup default flip in this PR is the fix." The audit hypothesis from
134+
the issue body ("client-retry across connection-close bypasses the
135+
probe") is **incorrect**: `PrevCommitTS` rides on the request envelope,
136+
not on the client TCP connection, so any retry that re-enters the
137+
adapter's `retryRedisWrite` loop (the path Jepsen exercises) carries
138+
the probe regardless of whether the underlying connection closed. The
139+
12-day green dedup-on streak on the identical workload demonstrates
140+
the existing mechanism closes the gap; no new design surface is
141+
required.
142+
143+
## R-references reused from the parent
144+
145+
This proposal does not introduce new risks; every risk from the parent
146+
applies unchanged. The two that change posture under default-on:
147+
148+
- **R5 (FSM determinism across a rolling upgrade) — discharged.** The
149+
probe code shipped on every node in production months ago. A node
150+
receiving a V2 `TxnMeta` with `prev_commit_ts != 0` is now uniformly
151+
capable of running the probe and producing the same apply outcome as
152+
every peer. The ordering constraint that gated this default flip is
153+
satisfied; the constraint stays documented so the same sequencing is
154+
reused for future probe extensions (e.g. extending dedup to a new
155+
command family).
156+
- **R6 (Operator rollback) — preserved.** Setting
157+
`ELASTICKV_REDIS_ONEPHASE_DEDUP=0` reverts to the pre-flip behaviour.
158+
This is a single-env-var change; no binary rollback required. The
159+
rollback contract is documented in the godoc on
160+
`WithOnePhaseTxnDedup` and in the comment at the env-var read site.
161+
162+
The parent's R1–R4 (result reconstruction on dedup no-op, retry-window
163+
window, MVCC visibility, store-side compaction) are unaffected by the
164+
default flip and do not need re-evaluation; the same code paths now
165+
just run in production by default.
166+
167+
## Test plan
168+
169+
- Unit: keep the existing `adapter/redis_*_dedup_test.go` suites green
170+
unchanged. Confirm that tests which currently rely on the gate being
171+
off either pass the explicit `WithOnePhaseTxnDedup(false)` option or
172+
`t.Setenv("ELASTICKV_REDIS_ONEPHASE_DEDUP", "0")` — audit and fix in
173+
the implementation commit.
174+
- Lint: `golangci-lint run` clean.
175+
- CI: the per-PR Jepsen run (`jepsen-test.yml`) and both scheduled
176+
stress workflows must stay green for the first run after merge.
177+
- Manual operator-check: a smoke run of the local Jepsen script
178+
(`./scripts/run-jepsen-local.sh`) without any env tweaks, confirming
179+
the gate is now on by default (look for the
180+
`r.onePhaseTxnDedup == true` branch in the adapter's startup log
181+
line, or inspect via the `RedisServer.onePhaseTxnDedup` field through
182+
a probe metric — if no metric exists today, log the active gate at
183+
`NewRedisServer` start; this is an optional sub-task).
184+
185+
## Out of scope
186+
187+
- **DynamoDB default-on.** Parallel work tracked by
188+
[`2026_06_03_partial_dynamodb_onephase_dedup.md`][dyn-doc]'s `M2`.
189+
The DynamoDB dedup-mode workflow has its own 7-day criterion that
190+
this proposal does not certify. A separate proposal will land the
191+
DynamoDB flip once its own M2 is met.
192+
- **S3, SQS dedup paths.** Not yet routed through one-phase reuse; the
193+
parent design's option-2 mechanism does not cover them. Out of scope
194+
for this PR; revisit when those adapters add reuse paths.
195+
- **Removing the env var.** Keep the opt-out env var indefinitely as a
196+
fast operator rollback. A future cleanup may remove it once
197+
operational confidence is high; not in this PR.
198+
199+
[dyn-doc]: 2026_06_03_partial_dynamodb_onephase_dedup.md
200+
201+
## Rollout
202+
203+
One PR, two commits:
204+
205+
1. **Doc commit**: add this file. Reviewable on its own — no behavior
206+
change.
207+
2. **Implementation commit**: flip the env-var sense in
208+
`adapter/redis.go`, update the comment and godoc, add the explicit
209+
`ELASTICKV_REDIS_ONEPHASE_DEDUP=0` to `jepsen-test-scheduled.yml`'s
210+
job env, fix any test that previously relied on the default-off
211+
posture (audit during the commit).
212+
213+
After merge: monitor the next 2–3 daily runs of both scheduled
214+
workflows. The dedup-mode workflow must stay green; the control
215+
workflow may or may not surface anomalies — both outcomes are
216+
informative. Roll back via env var (no binary revert) if anything
217+
unexpected appears.

0 commit comments

Comments
 (0)