Skip to content

Commit a202283

Browse files
authored
fix(kv): Composed-1 verifyOwnerFromSnapshot must routeKey-normalize before OwnerOf (issue #930) (#932)
**Closes #930.** `verifyOwnerFromSnapshot` was comparing the **raw mutation key** against the engine's **routing-key ranges**, which sit in a completely different lexicographical band. This caused every adapter write (DynamoDB, SQS, S3, Redis internal) that crossed into a non-default Raft group to be rejected with a spurious `ErrComposed1Violation`. ## 再現 (Issue #930) 1. Multi-group cluster を `--shardRanges ":<T3_KEY>=1,<T3_KEY>:=2"` で起動 2. M5a multi-table workload (PRs #911 / #916 / #924 / #925) を実行 3. Group 2 に routing されるテーブルへの全 CreateTable / Put / Get が以下の 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 が全 txn で `DynamoDB ResourceNotFoundException`、 Elle は `:empty-transaction-graph` を report ## Root cause `OwnerOf(rawKey)` の挙動: - `routes[].Start` と `routes[].End` は **routing key** (`!ddb|route|table|...`) の namespace - でも `mut.Key` は **raw user key** (`!ddb|meta|table|...`) - `!ddb|meta|...` (ASCII `m`=109) と `!ddb|route|...` (ASCII `r`=114) で 5 文字目がズレる - 結果: 全 `!ddb|meta|...` キーが lexicographically `T3_KEY=!ddb|route|...` より小さく、 group 1 へ "fall" させてしまう - 実際の routing (`engine.GetRoute(routeKey(rawKey))`) は正しく group 2 を返すので、 Dispatch と verify で不整合発生 ## Fix `snap.OwnerOf(routeKey(mut.Key))` で routing-key namespace で比較するよう normalize。 `ShardRouter.ResolveGroup` と `ShardStore.GetAt` が `engine.GetRoute(routeKey(rawKey))` するのと同じ semantic。 ## E2E 検証 修正後の `./scripts/run-jepsen-m5-local.sh` 実行結果: ``` results.edn: {:valid? true} ``` Workers が txn を正常完了、 read で先行 append を全て確認、 修正前の `ResourceNotFoundException` は完全に解消。 ## Caller audit (loop directive) `verifyOwnerFromSnapshot` は `verifyComposed1` の 2 箇所から呼ばれる: - observed-version check (line 618) - current-version check (line 626) 両方とも同じ mutation list を受けるので、 normalization は均等に適用される。 unexported なので外部 caller なし。 ## Test plan - [x] `./scripts/run-jepsen-m5-local.sh` — `{:valid? true}` - [ ] FSM レベルのユニットテスト追加 (follow-up — multi-group FSM verification の test infrastructure 整備とセット) - [x] `go build ./...` — 0 issues ## 関連 PR - #926 (`fix/m5a-local-host`): `--host 127.0.0.1` + `--shardRanges` default coverage — これも routing 問題だが、 PR #926 が解決するのは `Dispatch` 失敗パス。 本 PR は `verifyComposed1` の routing-key normalization。 両方が必要。 - #911 / #916 / #924 / #925: Composed-1 M5a 実装 (全部マージ済) - 親 design doc: `docs/design/2026_05_29_partial_composed1_cross_group_commit_guard.md` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed a route ownership verification issue that was incorrectly flagging certain adapter write operations as violations. The system now properly handles key comparisons during ownership checks, preventing spurious errors and improving system reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents d0d184b + 3807f76 commit a202283

3 files changed

Lines changed: 50 additions & 5 deletions

File tree

kv/fsm.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -695,11 +695,16 @@ func (f *kvFSM) verifyOwnerFromSnapshot(mutations []*pb.Mutation, snap RouteSnap
695695
if isTxnInternalKey(mut.Key) {
696696
continue
697697
}
698-
owner, found := snap.OwnerOf(mut.Key)
698+
// routeKey-normalize before OwnerOf so the gate routes the
699+
// same way as ShardRouter.ResolveGroup — raw adapter keys
700+
// and route catalog ranges live in different lex bands
701+
// (issue #930).
702+
rKey := routeKey(mut.Key)
703+
owner, found := snap.OwnerOf(rKey)
699704
if !found || owner != f.shardGroupID {
700705
return errors.Wrapf(ErrComposed1Violation,
701-
"%s-version v=%d: key %q owned by group %d (found=%v); this FSM serves group %d",
702-
phase, snapVer, mut.Key, owner, found, f.shardGroupID)
706+
"%s-version v=%d: key %q (routeKey %q) owned by group %d (found=%v); this FSM serves group %d",
707+
phase, snapVer, mut.Key, rKey, owner, found, f.shardGroupID)
703708
}
704709
}
705710
return nil

kv/fsm_composed1_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,3 +321,43 @@ func TestVerifyComposed1_AbortPhaseSkipsGate(t *testing.T) {
321321
require.NoError(t, fsm.verifyComposed1(abortReq),
322322
"ABORT requests must bypass the Composed-1 gate even with a non-zero ObservedRouteVersion pinned, so a route shift between PREPARE and ABORT cannot strand the txn's intent locks past lock-resolver TTL")
323323
}
324+
325+
// TestVerifyComposed1_DynamoMetaKeyNormalizedToRouteKeyBeforeOwnerOf
326+
// is the regression for issue #930. verifyOwnerFromSnapshot was
327+
// passing the raw adapter mutation key (e.g. "!ddb|meta|table|<seg>")
328+
// to OwnerOf, but the route catalog's ranges are keyed by routing
329+
// keys ("!ddb|route|table|<seg>"). "meta" (ASCII 'm'=109) sorts below
330+
// "route" ('r'=114), so every "!ddb|meta|..." key fell into the
331+
// empty-prefix range — the wrong group — and every cross-group
332+
// DynamoDB write was rejected with ErrComposed1Violation. The fix
333+
// normalizes the key via routeKey() before OwnerOf, matching what
334+
// ShardRouter.ResolveGroup and ShardStore.GetAt already do on the
335+
// engine side.
336+
//
337+
// Without the fix this test fails: OwnerOf returns group 1 for the
338+
// raw "!ddb|meta|table|anyseg" key (which is < the "!ddb|route|..."
339+
// boundary), so the FSM serving group 2 raises ErrComposed1Violation.
340+
// With routeKey() normalization the raw key maps to
341+
// "!ddb|route|table|anyseg" which falls into group 2's range and the
342+
// gate passes.
343+
func TestVerifyComposed1_DynamoMetaKeyNormalizedToRouteKeyBeforeOwnerOf(t *testing.T) {
344+
t.Parallel()
345+
346+
tableSegment := []byte("anyseg")
347+
routeBoundary := append([]byte("!ddb|route|table|"), tableSegment...)
348+
rawMetaKey := append([]byte("!ddb|meta|table|"), tableSegment...)
349+
350+
e := distribution.NewEngine()
351+
applyComposed1Snapshot(t, e, 1, []distribution.RouteDescriptor{
352+
// group 1 owns the empty-prefix half [<empty>, routeBoundary).
353+
// Without routeKey() normalization, rawMetaKey would fall in here.
354+
{RouteID: 100, Start: []byte(""), End: routeBoundary, GroupID: 1, State: distribution.RouteStateActive},
355+
// group 2 owns [routeBoundary, +inf). routeKey(rawMetaKey)
356+
// equals routeBoundary, so the normalized key lands here.
357+
{RouteID: 101, Start: routeBoundary, End: nil, GroupID: 2, State: distribution.RouteStateActive},
358+
})
359+
fsm := newComposed1FSM(t, e, 2) // this FSM serves group 2
360+
361+
require.NoError(t, fsm.verifyComposed1(commitTxnRequest(1, string(rawMetaKey))),
362+
"issue #930: verifyOwnerFromSnapshot must routeKey-normalize raw adapter keys before OwnerOf, so the gate routes the same way as ShardRouter.ResolveGroup; without normalization the raw \"!ddb|meta|...\" key sorts below the \"!ddb|route|...\" range and is falsely owned by group 1")
363+
}

kv/sharded_coordinator_composed1_retry_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,8 +758,8 @@ func (r *stubPartitionResolver) RecognisesPartitionedKey(key []byte) bool {
758758
// - Dispatch-entry auto-pin stamps ObservedRouteVersion =
759759
// engine.Version() on the request.
760760
// - FSM at gid R applies; verifyComposed1 calls route-history
761-
// OwnerOf on the raw mutation key against the engine's
762-
// snapshot.
761+
// OwnerOf on the routeKey()-normalized mutation key against
762+
// the engine's snapshot (issue #930 fix).
763763
// - Engine's snapshot says gid B (default byte-range owner),
764764
// not gid R.
765765
// - Mismatch → ErrComposed1Violation.

0 commit comments

Comments
 (0)