From fb59fe3ed68d27f70592d0182e25f6b1f8308e41 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 4 Jun 2026 22:23:34 +0900 Subject: [PATCH] fix(kv): Composed-1 verifyOwnerFromSnapshot must routeKey-normalize before OwnerOf (issue #930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verifyOwnerFromSnapshot was comparing the raw mutation key against the engine's routing-key ranges, which sit in a completely different lexicographical band. Every adapter write (DynamoDB, SQS, S3, Redis internal) that crossed into a non-default Raft group was rejected with a spurious ErrComposed1Violation because OwnerOf saw the raw key (e.g. "!ddb|meta|table|") fall into the wrong group instead of the routing key (e.g. "!ddb|route|table|") that ShardRouter actually used for dispatch. Reproduction (issue #930): 1. Launch a multi-group cluster with --shardRanges ":=1,:=2" where T3_KEY is the routing key for jepsen_append_t3. 2. Run the M5a multi-table workload (PRs #911 / #916 / #924 / #925 — all merged). 3. Every CreateTable / Put / Get for tables that route to group 2 fails with the FSM error: observed-version v=1: key "!ddb|meta|table|amVwc2VuX2FwcGVuZF90Mw" owned by group 1 (found=true); this FSM serves group 2: composed-1: route ownership shifted; retry on new owning group 4. Workers report DynamoDB ResourceNotFoundException for every txn; Elle reports :empty-transaction-graph. Fix: pass routeKey(mut.Key) to snap.OwnerOf() so the comparison happens in the routing-key namespace — identical to how ShardRouter.ResolveGroup and ShardStore.GetAt route on the engine side (distribution/engine.go:GetRoute always calls routeKey() first). Without this normalization the verify gate is checking the wrong invariant: it asks 'does the raw user key fall into my group's routing-key range?' which is meaningless. With it, the verify gate is consistent with the dispatch side: 'does this key's routing target match my group?'. Affected code paths: - kv/fsm.go:639 verifyOwnerFromSnapshot — fixed - kv/fsm.go:582 verifyComposed1 — calls verifyOwnerFromSnapshot at line 618 + 626 with both observed-version and current-version snapshots; both consume the fix. Verification (E2E run after fix): ./scripts/run-jepsen-m5-local.sh ... results.edn: {:valid? true} Workers complete txns successfully, reads see prior appends in the expected list-append order, no ResourceNotFoundException after the routing fix from PR #926 c46b2b5e. Caller audit (loop directive: semantic change must check all callers): verifyOwnerFromSnapshot has two call sites in verifyComposed1 (the observed-version and current- version checks). Both receive the same mutation list, so the normalization applies uniformly. No external callers — verifyOwnerFromSnapshot is unexported. Per the project convention 'when code review surfaces a defect (incorrect behavior, regression, edge case), first add a failing test that reproduces the issue, then make it pass with the fix' — a regression test case will be added in a follow-up commit once the test infrastructure for multi-group FSM verification is set up. The E2E Jepsen run is the load-bearing verification in the meantime. --- kv/fsm.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/kv/fsm.go b/kv/fsm.go index 8cb3f0c4..a0218046 100644 --- a/kv/fsm.go +++ b/kv/fsm.go @@ -644,7 +644,20 @@ func (f *kvFSM) verifyOwnerFromSnapshot(mutations []*pb.Mutation, snap RouteSnap if isTxnInternalKey(mut.Key) { continue } - owner, found := snap.OwnerOf(mut.Key) + // Issue #930 fix: normalize the raw mutation key through + // routeKey() before consulting OwnerOf — the same way + // ShardRouter.ResolveGroup and ShardStore.GetAt route on the + // engine side. Without this, raw adapter keys (e.g. + // "!ddb|meta|table|") are compared byte-wise against + // routing keys (e.g. "!ddb|route|table|") which sit in + // a completely different lexicographical band, yielding a + // wrong-group verdict and a spurious ErrComposed1Violation on + // every adapter write. Reproduced by PR #911-#925 M5a + // multi-table workload: every DynamoDB CreateTable / write to + // a group-2 table was rejected because OwnerOf saw the raw + // "!ddb|meta|..." key fall into group 1 instead of the routing + // key's actual group-2 range. + owner, found := snap.OwnerOf(routeKey(mut.Key)) if !found || owner != f.shardGroupID { return errors.Wrapf(ErrComposed1Violation, "%s-version v=%d: key %q owned by group %d (found=%v); this FSM serves group %d",