From 744ecbb5dba4e851a4dc6e0754f14ce636ab291c Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:04:24 +0900 Subject: [PATCH 1/8] docs: M5-3 proposed - SQS partitioned-FIFO reverse encoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0b M5-3 design doc. Lifts the ErrSQSEncodeUnsupportedPartitioned gate that M5-1 (#849) and M5-2 (#892) explicitly deferred. Adds: - sqsMessageRecord.Partition field + decoder plumbing. - Partitioned key-family emission (data/vis/byage/dedup) when meta.PartitionCount > 1. - Out-of-range and classic-vs-partitioned mismatch fail-closed guards. Group lock rows remain unemitted (carry-over from M5-2 — emitting any row falsely blocks the group). CLAUDE.md design-doc-first workflow: this doc lands before the implementation PR. --- ...3_proposed_sqs_partitioned_fifo_encoder.md | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md new file mode 100644 index 00000000..8872bfb4 --- /dev/null +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -0,0 +1,115 @@ +# SQS partitioned-FIFO reverse encoder (Phase 0b M5-3) — proposed + +**Status:** Proposed (no implementation yet). +**Parent:** [`2026_05_25_partial_snapshot_logical_encoder.md`](2026_05_25_partial_snapshot_logical_encoder.md) — this lifts the §"SQS" decision gate that M5-1 (`PR #849`) and M5-2 (`PR #892`) deferred for `partition_count > 1`. +**Predecessor on disk:** M5-1 emits `!sqs|queue|meta|`, `!sqs|queue|gen|`, `!sqs|queue|seq|`, `!sqs|msg|data|` for classic queues. M5-2 adds `!sqs|msg|vis|`, `!sqs|msg|byage|`, `!sqs|msg|dedup|`. Both reject `PartitionCount > 1` via `ErrSQSEncodeUnsupportedPartitioned` (`internal/backup/encode_sqs.go:162`); the M5-2 doc explicitly defers partitioned-FIFO support to "M5-3." + +## What needs to land + +For every queue with `partition_count > 1` in `_queue.json`, the encoder must read each message's partition assignment from the dump and emit the **partitioned** key family instead of the classic family: + +| Family | Classic shape (M5-1 / M5-2) | Partitioned shape (M5-3) | +| -------- | ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | +| `data` | `!sqs\|msg\|data\|` | `!sqs\|msg\|data\|p\|\|` | +| `vis` | `!sqs\|msg\|vis\|` | `!sqs\|msg\|vis\|p\|\|` | +| `byage` | `!sqs\|msg\|byage\|` | `!sqs\|msg\|byage\|p\|\|` | +| `dedup` | `!sqs\|msg\|dedup\|` | `!sqs\|msg\|dedup\|p\|\|` | +| `group` | (not emitted — see M5-2 §"families table" rationale) | (not emitted — same rationale) | + +> **Notation.** Pipe characters inside `…` are visual separators, not literal bytes; the only literal `|` bytes are inside the family prefix and the `|p|` discriminator. Constants in `adapter/sqs_keys.go`: `SqsPartitionedMsgDataPrefix` (line 41), `SqsPartitionedMsgVisPrefix`, `SqsPartitionedMsgByAgePrefix`, `SqsPartitionedMsgDedupPrefix`. Constructors: `sqsPartitionedMsgDataKey` (line 339), and the `*Vis,ByAge,Dedup,GroupKey` siblings in the same file. + +## Dump-format change (M5-1 decoder + encoder, NEW) + +`sqsMessageRecord` (`internal/backup/sqs.go:233`) does NOT currently carry a `partition` field. M5-3 adds it, plus a corresponding writer-side population in the decoder for `partition_count > 1` queues: + +```go +// sqsMessageRecord adds (in M5-3): +Partition uint32 `json:"partition,omitempty"` +``` + +`omitempty` is load-bearing — every classic-queue dump produced before M5-3 lands has no `partition` field, and the encoder MUST default to `partition=0` (the only valid value for `partition_count == 1`). New partitioned-queue dumps populate `partition` from the live key's partition trailer. + +**Backward compat:** a dump written before M5-3 (no `partition` field anywhere) round-trips through the M5-3 encoder unchanged — `omitempty` handles the read side, `meta.PartitionCount <= 1` already short-circuits the partitioned emit path. + +**Forward compat:** a dump written by an M5-3 decoder, then read by a pre-M5-3 encoder, would silently lose the `partition` field. The encoder is offline so cross-version replays are an operator-driven scenario; the parent-doc convention is to surface this via a fail-closed format-version bump if it ever matters. M5-3 keeps `format_version=1` because adding an optional field is backward compatible by JSON convention; if a later milestone needs to break compat it can bump the version then. + +## Decoder lift (M5-1 follow-up) + +The decoder's `decodeSQSMessageValue` (`internal/backup/sqs.go:719`) already runs after the partition trailer has been parsed by `sqsParsePartitionedMsgKey` (`sqs.go:600`). M5-3 plumbs the partition number through `sqsMessageRecord.Partition` and writes it to `messages.jsonl`. + +**Per-partition vs single-file layout.** Two candidate disk layouts: + +| Option | Layout | Pros | Cons | +| ------------------- | --------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- | +| **A (recommended)** | Single `messages.jsonl` per queue. Each line includes `"partition": `. | Minimal change to decoder; mirrors classic shape. Single file → atomic write + fsync stays simple. | Encoder must group-by-partition in memory (already loads the full file). | +| B | Per-partition directory: `sqs//partitions//messages.jsonl`. | Encoder can stream a partition at a time; smaller per-file working set on huge queues. | Decoder needs N writers, N fsyncs, partition-dir management. Breaks symmetry with classic dumps. | + +Option A is recommended for parity with the classic layout and to avoid a decoder rewrite. The encoder's `encodeQueueMessages` is already in-memory-buffered (loads full file → sorts → emits) so per-partition streaming wouldn't measurably help. + +## Encoder lift (M5-1 + M5-2) + +Three changes to `internal/backup/encode_sqs.go`: + +1. **Drop `ErrSQSEncodeUnsupportedPartitioned`.** Remove the `meta.PartitionCount > 1` gate at line 162. +2. **Branch on `PartitionCount`.** When `> 1`, use partitioned key constructors (duplicated from `adapter/sqs_keys.go` following the established M3b-3 GSI pattern). When `<= 1`, classic constructors as today. +3. **Group-by-partition before emit.** Sort messages by `(partition, send_timestamp_millis, message_id)` so per-partition order is stable across runs — required for byte-identical re-encodes. + +Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): + +1. Partitioned `vis` constructor → `!sqs|msg|vis|p|...`. +2. Partitioned `byage` constructor → `!sqs|msg|byage|p|...`. +3. Partitioned `dedup` constructor → `!sqs|msg|dedup|p|...`. Note the partitioned shape adds a `` segment before `` (per `adapter/sqs_keys.go:sqsPartitionedMsgDedupKey` line N+50ish); the classic shape has only ``. This means `message_group_id` is now load-bearing for dedup-row construction on FIFO partitioned queues — but the existing `messages.jsonl` already carries it as `message_group_id`. + +## Validation invariants (fail-closed) + +The encoder fails closed with the existing per-adapter sentinels on: + +- `meta.PartitionCount > 1` AND any message has `Partition == 0` AND the dump's record count for partition 0 doesn't match the live partition assignment. (Detectable only if the encoder can recompute the partition; deferred to a self-test invariant rather than a runtime check.) +- `meta.PartitionCount > 1` AND any message's `Partition >= meta.PartitionCount` — out-of-range partition number, dump is malformed. New sentinel `ErrSQSEncodeOutOfRangePartition`. +- `meta.PartitionCount == 1` (classic) AND any message has `Partition != 0` — dump is internally inconsistent. Reuses `ErrSQSInvalidMessage`. + +## Decision gate: full reconstruction vs lazy rebuild (carry-over from M5-2) + +M5-2's "full reconstruction" gate applies unchanged to M5-3 partitioned queues. The recommendation is the same — emit `vis` + `byage` + `dedup` inline during the per-message walk; do NOT emit `group` rows. Cost remains O(messages-in-dump); no extra disk read or Raft round-trip. + +## Out of scope (deferred) + +- **Cross-partition rebalancing.** A partition count change between dump and restore would invalidate every message's partition assignment. M5-3 forbids this — `meta.PartitionCount` must match the input dump exactly. A future milestone can add a `--repartition` flag that re-hashes message IDs into the target partition count. +- **In-flight cross-partition receives.** Same `vis`-is-zero rule as M5-2; restored messages are visible. Documented in the encoder header. +- **Group lock rows.** Same prohibition as M5-2 — emitting any row falsely blocks the group permanently. M5-3 inherits the rule. + +## Files to add / modify (M5-3 implementation slice) + +``` +internal/backup/sqs.go # sqsMessageRecord +Partition; decodeSQSMessageValue plumbs partition +internal/backup/encode_sqs.go # drop ErrSQSEncodeUnsupportedPartitioned; branch on PartitionCount +internal/backup/encode_sqs_side.go # add partitioned vis/byage/dedup constructors + emit +internal/backup/encode_sqs_test.go # round-trip partitioned-FIFO fixture (2 partitions × 3 messages) +internal/backup/encode_sqs_side_test.go # cross-check partitioned vis/byage/dedup vs live constructors +internal/backup/sqs_test.go # decoder round-trip with partition field populated +``` + +## Milestones (within M5-3) + +The slice ships as a single PR — the decoder format change and encoder partition branch are tightly coupled (a partial landing would either reject all M5-3 dumps at the new encoder or break old encoders against new dumps). + +## Test plan + +| Test | Verifies | +| ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | +| `TestSQSEncodePartitionedQueueRoundTrip` | `partition_count=2`, 3 messages across both partitions → all data + side records emitted with `|p|` prefix | +| `TestSQSEncodePartitionedDedupBuildsGroupSegment` | FIFO partitioned dedup row's `` matches `message_group_id` from `messages.jsonl` | +| `TestSQSEncodeRejectsOutOfRangePartition` | message with `Partition >= meta.PartitionCount` → `ErrSQSEncodeOutOfRangePartition` | +| `TestSQSEncodeRejectsNonZeroPartitionOnClassicQueue` | `PartitionCount=1` but message has `Partition=2` → `ErrSQSInvalidMessage` | +| `TestSQSEncodeLegacyDumpsWithoutPartitionStillRoundTrip` | a pre-M5-3 `messages.jsonl` with no `partition` field round-trips through M5-3 encoder unchanged | +| `TestSQSEncodePartitionedSideRecordsByteCrossCheckLiveAdapter` | M5-2-style cross-check: partitioned `vis|p|` / `byage|p|` / `dedup|p|` bytes equal `sqsPartitionedMsg{...}Key(...)` | + +## References + +- Parent: `2026_05_25_partial_snapshot_logical_encoder.md` §"SQS" +- M5-2 doc (decision gate template, classic side records): `2026_05_30_proposed_sqs_side_record_derivation.md` +- M5-1 PR: #849 +- M5-2 PR: #892 +- Live partitioned constructors: `adapter/sqs_keys.go:337+` (`sqsPartitionedMsgDataKey` and siblings) +- Existing partitioned dispatch (cross-classic-partitioned routing): `adapter/sqs_keys_dispatch.go` +- Existing gate in encoder: `internal/backup/encode_sqs.go:162` (`ErrSQSEncodeUnsupportedPartitioned`) From 48bcab751221545442a8aa9dea3b45b0d8870437 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:10:59 +0900 Subject: [PATCH 2/8] docs: M5-3 v2 - codex P2 doc fixes (terminator + parser plumbing) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two doc fixes from the first-round review: 1. Codex P2 (L16): the partitioned-dedup key shape was missing the sqsPartitionedQueueTerminator between and . The live sqsPartitionedMsgDedupKey (adapter/sqs_keys.go:389) emits this delimiter because base64.RawURLEncoding has no padding, so back-to-back raw-base64 segments would let distinct (group,dedup) pairs collapse onto the same key. Add the delimiter to the table + a notation paragraph explaining why every partitioned key carries the terminator after AND why dedup additionally carries one between group/dedup. 2. Codex P2 (L38): v1 said the partition is 'plumbed through sqsMessageRecord.Partition' but parseSQSMessageDataKey / parseSQSPartitionedQueueAndTrailer currently return only the encoded queue (with the partition u32 parsed-then-discarded inside parseSQSPartitionedQueueAndTrailer). v2 explicitly requires extending both signatures to return the partition, and clarifies the message value itself never carried partition — the key is the source of truth. --- ...3_proposed_sqs_partitioned_fifo_encoder.md | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index 8872bfb4..3bd265ed 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -13,10 +13,10 @@ For every queue with `partition_count > 1` in `_queue.json`, the encoder must re | `data` | `!sqs\|msg\|data\|` | `!sqs\|msg\|data\|p\|\|` | | `vis` | `!sqs\|msg\|vis\|` | `!sqs\|msg\|vis\|p\|\|` | | `byage` | `!sqs\|msg\|byage\|` | `!sqs\|msg\|byage\|p\|\|` | -| `dedup` | `!sqs\|msg\|dedup\|` | `!sqs\|msg\|dedup\|p\|\|` | +| `dedup` | `!sqs\|msg\|dedup\|` | `!sqs\|msg\|dedup\|p\|\|\|` | | `group` | (not emitted — see M5-2 §"families table" rationale) | (not emitted — same rationale) | -> **Notation.** Pipe characters inside `…` are visual separators, not literal bytes; the only literal `|` bytes are inside the family prefix and the `|p|` discriminator. Constants in `adapter/sqs_keys.go`: `SqsPartitionedMsgDataPrefix` (line 41), `SqsPartitionedMsgVisPrefix`, `SqsPartitionedMsgByAgePrefix`, `SqsPartitionedMsgDedupPrefix`. Constructors: `sqsPartitionedMsgDataKey` (line 339), and the `*Vis,ByAge,Dedup,GroupKey` siblings in the same file. +> **Notation.** Pipe characters inside `…` are visual separators, not literal bytes; the only literal `|` bytes are (1) inside the family prefix, (2) the `|p|` partitioned-key discriminator, (3) the `sqsPartitionedQueueTerminator='|'` byte appended after `` in every partitioned key (see `adapter/sqs_keys.go:82`), and (4) for the partitioned dedup family ONLY, an additional `sqsPartitionedQueueTerminator` between `` and `` — the live `sqsPartitionedMsgDedupKey` (`adapter/sqs_keys.go:389`) emits this delimiter because `encodeSQSSegment` uses `base64.RawURLEncoding` (no padding) and back-to-back raw-base64 segments are ambiguous, so distinct `(groupID, dedupID)` pairs could collapse onto the same key (CodeRabbit major PR #732 round 6 + codex P2 found this missing from v1 of this doc). Constants in `adapter/sqs_keys.go`: `SqsPartitionedMsgDataPrefix` (line 41), `SqsPartitionedMsgVisPrefix`, `SqsPartitionedMsgByAgePrefix`, `SqsPartitionedMsgDedupPrefix`. Constructors: `sqsPartitionedMsgDataKey` (line 339), and the `*Vis,ByAge,Dedup,GroupKey` siblings in the same file. ## Dump-format change (M5-1 decoder + encoder, NEW) @@ -35,7 +35,26 @@ Partition uint32 `json:"partition,omitempty"` ## Decoder lift (M5-1 follow-up) -The decoder's `decodeSQSMessageValue` (`internal/backup/sqs.go:719`) already runs after the partition trailer has been parsed by `sqsParsePartitionedMsgKey` (`sqs.go:600`). M5-3 plumbs the partition number through `sqsMessageRecord.Partition` and writes it to `messages.jsonl`. +The current key parser only returns the encoded queue segment: + +```go +// internal/backup/sqs.go:534 +func parseSQSMessageDataKey(key []byte) (string, error) +// internal/backup/sqs.go:611 +func parseSQSPartitionedQueueAndTrailer(rest string, hasMsgID bool, originalKey []byte) (string, error) +``` + +The partition `uint32` is parsed inside `parseSQSPartitionedQueueAndTrailer` (the trailer carries `` immediately after the queue segment + terminator) but is currently DISCARDED — codex P2 found this in the v1 review. M5-3 MUST extend both signatures to return the partition: + +```go +// internal/backup/sqs.go (M5-3): +func parseSQSMessageDataKey(key []byte) (encQueue string, partition uint32, isPartitioned bool, err error) +func parseSQSPartitionedQueueAndTrailer(rest string, hasMsgID bool, originalKey []byte) (encQueue string, partition uint32, err error) +``` + +(`isPartitioned` is needed because `parseSQSMessageDataKey` is also the classic dispatcher — caller can branch on it without re-checking the `|p|` discriminator. For the classic path `partition` is always 0.) + +`decodeSQSMessageValue` then receives the parsed `partition` from its caller (the FSM-apply / dump-walker), populates `sqsMessageRecord.Partition`, and `writeMessagesJSONL` serializes it under the new JSON field. The live message value itself never contained the partition (the value pair is ``, and partition routing lives in the KEY); M5-3 closes that gap by passing through the key-derived partition rather than re-deriving it on the decoder side. **Per-partition vs single-file layout.** Two candidate disk layouts: From 1d1561e28188ab56e2a7cfe97ed817356b299ff9 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:18:56 +0900 Subject: [PATCH 3/8] docs: M5-3 v3 - partition presence + sort key + parser plumbing (codex P1 + claude review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from the v2 review: 1. Codex P1 (L27) + Gemini #1: uint32+omitempty made partition-0 indistinguishable from absent. Replace with *uint32: nil = pre-M5-3 dump or never-captured, non-nil = explicitly captured (incl. 0). New ErrSQSEncodeMissingPartition fail-closed sentinel when partition_count>1 AND rec.Partition==nil. Decision matrix table added to make all 6 (partition_count × Partition shape) cases explicit. 2. Gemini #2 (sort): include sequence_number in the per-partition sort key. Now (partition, send_timestamp_millis, sequence_number, message_id) — matches the classic path's tiebreakers. 3. Claude review (call-site plumbing): explicit notes that HandleMessageData wires rec.Partition = &partition only when isPartitioned; decodeSQSMessageValue keeps its current (value)->(*record, error) signature; parseSQSGenericKey wrapper updated mechanically to discard the new partition return. New test pinned: TestSQSEncodeRejectsMissingPartitionOnPartitionedQueue. --- ...3_proposed_sqs_partitioned_fifo_encoder.md | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index 3bd265ed..6e1dbe41 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -20,16 +20,29 @@ For every queue with `partition_count > 1` in `_queue.json`, the encoder must re ## Dump-format change (M5-1 decoder + encoder, NEW) -`sqsMessageRecord` (`internal/backup/sqs.go:233`) does NOT currently carry a `partition` field. M5-3 adds it, plus a corresponding writer-side population in the decoder for `partition_count > 1` queues: +`sqsMessageRecord` (`internal/backup/sqs.go:233`) does NOT currently carry a `partition` field. M5-3 adds it as a **pointer** (`*uint32`) so the encoder can distinguish "partition was captured and is 0" from "partition field absent in the dump": ```go // sqsMessageRecord adds (in M5-3): -Partition uint32 `json:"partition,omitempty"` +Partition *uint32 `json:"partition,omitempty"` ``` -`omitempty` is load-bearing — every classic-queue dump produced before M5-3 lands has no `partition` field, and the encoder MUST default to `partition=0` (the only valid value for `partition_count == 1`). New partitioned-queue dumps populate `partition` from the live key's partition trailer. +**Why `*uint32` instead of `uint32`** (codex P1 / gemini #914 v2): partition 0 is a valid partition (it's the only valid partition for `partition_count == 1` classic queues AND a legitimate partition for FIFO-hash-routed messages in a multi-partition queue). With a plain `uint32 + omitempty`, a partition-0 message serializes as `{}` — identical on the wire to a legacy classic-queue message that was never partition-aware. A future operator who manually flips `_queue.json`'s `partition_count` from 1 to N and replays the old dump would have every message silently land in partition 0, breaking the FIFO group-hash routing invariant without an error. The pointer form makes "partition was captured" detectable. -**Backward compat:** a dump written before M5-3 (no `partition` field anywhere) round-trips through the M5-3 encoder unchanged — `omitempty` handles the read side, `meta.PartitionCount <= 1` already short-circuits the partitioned emit path. +`omitempty` stays on the JSON tag so the dump for a classic-queue message (no partition concept) omits the field entirely; legacy `format_version=1` dumps produced before M5-3 deserialize with `Partition == nil`, which the encoder then handles by branch: + +| Manifest | `Partition` nil? | Encoder behavior | +| ---------------------------------------- | ----------------- | ---------------------------------------------------------------------------------------------------- | +| `partition_count == 1` (classic) | nil | Emit classic-shape keys; this is the legacy / forward-compatible path. | +| `partition_count == 1` (classic) | non-nil + `== 0` | Emit classic-shape keys; allowed (a freshly-decoded classic dump writes `0` explicitly under M5-3). | +| `partition_count == 1` (classic) | non-nil + `!= 0` | Fail-closed `ErrSQSInvalidMessage` (dump is internally inconsistent). | +| `partition_count > 1` (partitioned) | **nil** | **Fail-closed `ErrSQSEncodeMissingPartition`** — pre-M5-3 dumps cannot be replayed under M5-3's lifted gate; the operator must re-decode with an M5-3 decoder. | +| `partition_count > 1` (partitioned) | non-nil + in-range| Emit partitioned-shape keys with `*rec.Partition`. | +| `partition_count > 1` (partitioned) | non-nil + out-of-range | Fail-closed `ErrSQSEncodeOutOfRangePartition`. | + +New sentinel `ErrSQSEncodeMissingPartition` (alongside `ErrSQSEncodeOutOfRangePartition` from v1) covers the legacy-dump-under-lifted-gate case. + +**Backward compat:** a dump written before M5-3 (no `partition` field anywhere) for a classic queue (`partition_count == 1`) round-trips through the M5-3 encoder unchanged — `*uint32` decodes as `nil`, the classic branch takes the `nil → classic-shape keys` path. **Forward compat:** a dump written by an M5-3 decoder, then read by a pre-M5-3 encoder, would silently lose the `partition` field. The encoder is offline so cross-version replays are an operator-driven scenario; the parent-doc convention is to surface this via a fail-closed format-version bump if it ever matters. M5-3 keeps `format_version=1` because adding an optional field is backward compatible by JSON convention; if a later milestone needs to break compat it can bump the version then. @@ -54,7 +67,13 @@ func parseSQSPartitionedQueueAndTrailer(rest string, hasMsgID bool, originalKey (`isPartitioned` is needed because `parseSQSMessageDataKey` is also the classic dispatcher — caller can branch on it without re-checking the `|p|` discriminator. For the classic path `partition` is always 0.) -`decodeSQSMessageValue` then receives the parsed `partition` from its caller (the FSM-apply / dump-walker), populates `sqsMessageRecord.Partition`, and `writeMessagesJSONL` serializes it under the new JSON field. The live message value itself never contained the partition (the value pair is ``, and partition routing lives in the KEY); M5-3 closes that gap by passing through the key-derived partition rather than re-deriving it on the decoder side. +`decodeSQSMessageValue` then receives the parsed `partition` from its caller (`HandleMessageData`, `internal/backup/sqs.go:341`), populates `sqsMessageRecord.Partition` as a `*uint32`, and `writeMessagesJSONL` serializes it under the new JSON field. The live message value itself never contained the partition (the value pair is ``, and partition routing lives in the KEY); M5-3 closes that gap by passing through the key-derived partition rather than re-deriving it on the decoder side. + +**Specific call-site updates** (claude v2 review caught these — make the impl PR mechanically faithful): + +- `HandleMessageData` (`sqs.go:341`): receive `(encQueue, partition uint32, isPartitioned bool, err)`, decode the value bytes, then set `rec.Partition = &partition` ONLY when `isPartitioned` is true. Classic-key call site keeps `rec.Partition = nil` so the no-partition-concept case round-trips as a legacy dump. +- `decodeSQSMessageValue` keeps its current `(value []byte) → (*sqsMessageRecord, error)` signature — partition wiring happens at the call site, not inside the decoder, because the decoder never sees the key. +- `parseSQSGenericKey` (`sqs.go:571`) — wraps `parseSQSPartitionedQueueAndTrailer` and is called by `HandleSideRecord` for `vis`/`byage`/`dedup` value handling (`sqs.go:367`). Side-record handlers route to `_internals/` by queue and don't need partition, so the new partition return from `parseSQSPartitionedQueueAndTrailer` is discarded inside `parseSQSGenericKey`. The signature change is mechanical; impl PR must touch the wrapper to compile. **Per-partition vs single-file layout.** Two candidate disk layouts: @@ -71,7 +90,7 @@ Three changes to `internal/backup/encode_sqs.go`: 1. **Drop `ErrSQSEncodeUnsupportedPartitioned`.** Remove the `meta.PartitionCount > 1` gate at line 162. 2. **Branch on `PartitionCount`.** When `> 1`, use partitioned key constructors (duplicated from `adapter/sqs_keys.go` following the established M3b-3 GSI pattern). When `<= 1`, classic constructors as today. -3. **Group-by-partition before emit.** Sort messages by `(partition, send_timestamp_millis, message_id)` so per-partition order is stable across runs — required for byte-identical re-encodes. +3. **Group-by-partition before emit.** Sort messages by `(partition, send_timestamp_millis, sequence_number, message_id)` so per-partition order is stable across runs — required for byte-identical re-encodes. `sequence_number` is the definitive tiebreaker between messages that share a send timestamp (burst case); `message_id` is the last-resort tiebreaker. Gemini suggested this in the v2 review — the classic path uses the same four-field sort, so the partitioned path adds `partition` as the leading key. Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): @@ -81,11 +100,13 @@ Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): ## Validation invariants (fail-closed) -The encoder fails closed with the existing per-adapter sentinels on: +The full decision matrix lives in the table under §"Dump-format change." The encoder fails closed with these sentinels: + +- `meta.PartitionCount > 1` AND `rec.Partition == nil` (pre-M5-3 dump under lifted gate, or M5-3 decoder bug) → **new sentinel `ErrSQSEncodeMissingPartition`**. The operator must re-decode with an M5-3 decoder; replaying a legacy dump into a partitioned queue would silently move every message to partition 0 (codex P1 / gemini #914 v2). +- `meta.PartitionCount > 1` AND `*rec.Partition >= meta.PartitionCount` → **new sentinel `ErrSQSEncodeOutOfRangePartition`** (out-of-range partition number, dump is malformed). +- `meta.PartitionCount == 1` (classic) AND `rec.Partition != nil && *rec.Partition != 0` → reuses **`ErrSQSInvalidMessage`** (dump is internally inconsistent). -- `meta.PartitionCount > 1` AND any message has `Partition == 0` AND the dump's record count for partition 0 doesn't match the live partition assignment. (Detectable only if the encoder can recompute the partition; deferred to a self-test invariant rather than a runtime check.) -- `meta.PartitionCount > 1` AND any message's `Partition >= meta.PartitionCount` — out-of-range partition number, dump is malformed. New sentinel `ErrSQSEncodeOutOfRangePartition`. -- `meta.PartitionCount == 1` (classic) AND any message has `Partition != 0` — dump is internally inconsistent. Reuses `ErrSQSInvalidMessage`. +Per-partition-record-count integrity (detecting "all messages in partition 0, none in partition 1..N-1") cannot be checked from the dump alone — the live partition assignment is `partitionFor(messageGroupID) → uint32` and the encoder doesn't run that hash. The self-test loop catches it because a re-decode would write the messages into the same partitions the encoder used — but only if the encoder respects the per-message partition, which is what the new fail-closed gates above ensure. ## Decision gate: full reconstruction vs lazy rebuild (carry-over from M5-2) @@ -118,7 +139,8 @@ The slice ships as a single PR — the decoder format change and encoder partiti | ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | | `TestSQSEncodePartitionedQueueRoundTrip` | `partition_count=2`, 3 messages across both partitions → all data + side records emitted with `|p|` prefix | | `TestSQSEncodePartitionedDedupBuildsGroupSegment` | FIFO partitioned dedup row's `` matches `message_group_id` from `messages.jsonl` | -| `TestSQSEncodeRejectsOutOfRangePartition` | message with `Partition >= meta.PartitionCount` → `ErrSQSEncodeOutOfRangePartition` | +| `TestSQSEncodeRejectsMissingPartitionOnPartitionedQueue` | `partition_count=2`, message with `Partition == nil` (legacy dump shape) → `ErrSQSEncodeMissingPartition` | +| `TestSQSEncodeRejectsOutOfRangePartition` | message with `*Partition >= meta.PartitionCount` → `ErrSQSEncodeOutOfRangePartition` | | `TestSQSEncodeRejectsNonZeroPartitionOnClassicQueue` | `PartitionCount=1` but message has `Partition=2` → `ErrSQSInvalidMessage` | | `TestSQSEncodeLegacyDumpsWithoutPartitionStillRoundTrip` | a pre-M5-3 `messages.jsonl` with no `partition` field round-trips through M5-3 encoder unchanged | | `TestSQSEncodePartitionedSideRecordsByteCrossCheckLiveAdapter` | M5-2-style cross-check: partitioned `vis|p|` / `byage|p|` / `dedup|p|` bytes equal `sqsPartitionedMsg{...}Key(...)` | From 810634d5960bc47ee04d0079cb4208b0042a3eef Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:32:25 +0900 Subject: [PATCH 4/8] docs: M5-3 v4 - 3-field sort + value return type + side-record dispatcher (claude v914 v3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the v3 claude review, all editorial / mechanical: 1. Minor #1: "classic path four-field sort" wording at L93 was wrong. The live sortMessagesForEmit (sqs.go:815) is a 3-field tuple (send_timestamp_millis, sequence_number, message_id). Partitioned path prepends partition as the leading key. Reworded. 2. Minor #2: decodeSQSMessageValue return type at L75 — live sig is (value []byte) (sqsMessageRecord, error), a value not a pointer. Doc previously said *sqsMessageRecord — corrected. 3. Low #3: encodeMessageSideRecords partition param plumbing now spelled out explicitly in the §"Encoder lift" side-record section. Branches on partition != nil rather than re-reading meta.PartitionCount. 4. Low #4: addMessage / partitioned dispatch named. v4 threads partition *uint32 through addMessage (rather than spawning a peer addPartitionedMessage) so addMessage + encodeMessageSideRecords share the same dispatch source. 5. Editorial: Files-to-modify comment for sqs.go now lists all four touched functions (sqsMessageRecord +Partition, parseSQSMessageDataKey + parseSQSPartitionedQueueAndTrailer new returns, HandleMessageData wiring, parseSQSGenericKey wrapper discard). --- ..._06_03_proposed_sqs_partitioned_fifo_encoder.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index 6e1dbe41..a2bcc11f 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -72,7 +72,7 @@ func parseSQSPartitionedQueueAndTrailer(rest string, hasMsgID bool, originalKey **Specific call-site updates** (claude v2 review caught these — make the impl PR mechanically faithful): - `HandleMessageData` (`sqs.go:341`): receive `(encQueue, partition uint32, isPartitioned bool, err)`, decode the value bytes, then set `rec.Partition = &partition` ONLY when `isPartitioned` is true. Classic-key call site keeps `rec.Partition = nil` so the no-partition-concept case round-trips as a legacy dump. -- `decodeSQSMessageValue` keeps its current `(value []byte) → (*sqsMessageRecord, error)` signature — partition wiring happens at the call site, not inside the decoder, because the decoder never sees the key. +- `decodeSQSMessageValue` (`sqs.go:719`) keeps its current `(value []byte) (sqsMessageRecord, error)` signature — note the return is a value, not a pointer (claude v3 v914 caught the earlier doc typo). Partition wiring happens at the call site, not inside the decoder, because the decoder never sees the key. - `parseSQSGenericKey` (`sqs.go:571`) — wraps `parseSQSPartitionedQueueAndTrailer` and is called by `HandleSideRecord` for `vis`/`byage`/`dedup` value handling (`sqs.go:367`). Side-record handlers route to `_internals/` by queue and don't need partition, so the new partition return from `parseSQSPartitionedQueueAndTrailer` is discarded inside `parseSQSGenericKey`. The signature change is mechanical; impl PR must touch the wrapper to compile. **Per-partition vs single-file layout.** Two candidate disk layouts: @@ -90,7 +90,9 @@ Three changes to `internal/backup/encode_sqs.go`: 1. **Drop `ErrSQSEncodeUnsupportedPartitioned`.** Remove the `meta.PartitionCount > 1` gate at line 162. 2. **Branch on `PartitionCount`.** When `> 1`, use partitioned key constructors (duplicated from `adapter/sqs_keys.go` following the established M3b-3 GSI pattern). When `<= 1`, classic constructors as today. -3. **Group-by-partition before emit.** Sort messages by `(partition, send_timestamp_millis, sequence_number, message_id)` so per-partition order is stable across runs — required for byte-identical re-encodes. `sequence_number` is the definitive tiebreaker between messages that share a send timestamp (burst case); `message_id` is the last-resort tiebreaker. Gemini suggested this in the v2 review — the classic path uses the same four-field sort, so the partitioned path adds `partition` as the leading key. +3. **Group-by-partition before emit.** Sort messages by `(partition, send_timestamp_millis, sequence_number, message_id)` so per-partition order is stable across runs — required for byte-identical re-encodes. `sequence_number` is the definitive tiebreaker between messages that share a send timestamp (burst case); `message_id` is the last-resort tiebreaker. Gemini suggested this in the v2 review; claude v3 corrected the wording — the live `sortMessagesForEmit` (`sqs.go:815`) is a **3-field** tuple `(send_timestamp_millis, sequence_number, message_id)`, and the partitioned path prepends `partition` as the leading key. + +4. **Per-message dispatch.** `addMessage` (`encode_sqs.go:241`) currently calls `sqsMsgDataKeyBytes(queueName, sqsRestoreGeneration, rec.MessageID)` at line 268. M5-3 threads a `partition *uint32` parameter through `addMessage`: when non-nil, it calls a new `sqsPartitionedMsgDataKeyBytes(queueName, *partition, sqsRestoreGeneration, rec.MessageID)`; when nil, the existing classic constructor. Adding the parameter (rather than a peer `addPartitionedMessage`) keeps the side-record dispatch in lockstep — `encodeMessageSideRecords` runs on the same record and needs the same partition info. Claude v3 noted that the doc previously left this implicit. Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): @@ -98,6 +100,8 @@ Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): 2. Partitioned `byage` constructor → `!sqs|msg|byage|p|...`. 3. Partitioned `dedup` constructor → `!sqs|msg|dedup|p|...`. Note the partitioned shape adds a `` segment before `` (per `adapter/sqs_keys.go:sqsPartitionedMsgDedupKey` line N+50ish); the classic shape has only ``. This means `message_group_id` is now load-bearing for dedup-row construction on FIFO partitioned queues — but the existing `messages.jsonl` already carries it as `message_group_id`. +`encodeMessageSideRecords` (the per-message side-record dispatcher in `encode_sqs_side.go`) gains the same `partition *uint32` parameter as `addMessage` so the partitioned constructors can be selected per message. Branching by `partition != nil` (rather than re-reading `meta.PartitionCount`) keeps the call site one decision deep and makes mixed-mode bugs impossible — a classic-mode dump can never accidentally call a partitioned constructor because `rec.Partition` is enforced nil for `PartitionCount == 1` (the new fail-closed gate in §"Validation invariants" forbids any other shape). Claude v3 noted that v3 didn't spell this out. + ## Validation invariants (fail-closed) The full decision matrix lives in the table under §"Dump-format change." The encoder fails closed with these sentinels: @@ -121,9 +125,9 @@ M5-2's "full reconstruction" gate applies unchanged to M5-3 partitioned queues. ## Files to add / modify (M5-3 implementation slice) ``` -internal/backup/sqs.go # sqsMessageRecord +Partition; decodeSQSMessageValue plumbs partition -internal/backup/encode_sqs.go # drop ErrSQSEncodeUnsupportedPartitioned; branch on PartitionCount -internal/backup/encode_sqs_side.go # add partitioned vis/byage/dedup constructors + emit +internal/backup/sqs.go # sqsMessageRecord +Partition *uint32; parseSQSMessageDataKey + parseSQSPartitionedQueueAndTrailer new (partition uint32) return values; HandleMessageData wires rec.Partition = &partition only when isPartitioned; parseSQSGenericKey wrapper discards the new return (claude v3 v914) +internal/backup/encode_sqs.go # drop ErrSQSEncodeUnsupportedPartitioned; addMessage takes partition *uint32 + branches between sqsMsgDataKeyBytes and the new sqsPartitionedMsgDataKeyBytes +internal/backup/encode_sqs_side.go # encodeMessageSideRecords takes partition *uint32; add partitioned vis/byage/dedup constructors + emit internal/backup/encode_sqs_test.go # round-trip partitioned-FIFO fixture (2 partitions × 3 messages) internal/backup/encode_sqs_side_test.go # cross-check partitioned vis/byage/dedup vs live constructors internal/backup/sqs_test.go # decoder round-trip with partition field populated From 8886c22fd7c5d0fc80c29261349c7133efef8974 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:40:51 +0900 Subject: [PATCH 5/8] docs: M5-3 v5 - perQueue routing gate + addSideRecords name (codex P2 + claude v914 v4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the v4 review: 1. Codex P2 v914 v4 (L110): For FIFO partitioned queues with FifoThroughputLimit == "perQueue", the live partitionFor at adapter/sqs_partitioning.go:71-72 forces every group to partition 0 regardless of PartitionCount, and ReceiveMessage only scans the partition-0 lane (adapter/sqs_keys_dispatch.go:125-126). The v4 invariants only checked *rec.Partition < PartitionCount, so a dump line with partition 1..N-1 would be accepted and restored onto |p| lanes the live receive never visits — silent data loss on first read. v5 adds new sentinel ErrSQSEncodePartitionRoutingMismatch and the TestSQSEncodeRejectsNonzeroPartitionOnPerQueueHTFIFO regression. Also documents the effectivePartitionCount(meta) helper concept so the per-gate inline checks stay one-line clean. 2. Claude v914 v4 (Low-Medium): The per-message side-record dispatcher is named addSideRecords (encode_sqs_side.go:133), called from encode_sqs.go:214 as e.addSideRecords(b, meta.Name, &meta, &records[i]). v4 used encodeMessageSideRecords in three places — that function does not exist. Renamed throughout the doc. The perQueue gate is semantic-changing (new fail-closed branch). Callers of the new helper effectivePartitionCount: only addMessage and addSideRecords in M5-3's encoder. The validation site itself is the encoder's per-message loop, which is the only place that has both meta and the candidate rec.Partition. --- ...2026_06_03_proposed_sqs_partitioned_fifo_encoder.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index a2bcc11f..3683ea0b 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -92,7 +92,7 @@ Three changes to `internal/backup/encode_sqs.go`: 2. **Branch on `PartitionCount`.** When `> 1`, use partitioned key constructors (duplicated from `adapter/sqs_keys.go` following the established M3b-3 GSI pattern). When `<= 1`, classic constructors as today. 3. **Group-by-partition before emit.** Sort messages by `(partition, send_timestamp_millis, sequence_number, message_id)` so per-partition order is stable across runs — required for byte-identical re-encodes. `sequence_number` is the definitive tiebreaker between messages that share a send timestamp (burst case); `message_id` is the last-resort tiebreaker. Gemini suggested this in the v2 review; claude v3 corrected the wording — the live `sortMessagesForEmit` (`sqs.go:815`) is a **3-field** tuple `(send_timestamp_millis, sequence_number, message_id)`, and the partitioned path prepends `partition` as the leading key. -4. **Per-message dispatch.** `addMessage` (`encode_sqs.go:241`) currently calls `sqsMsgDataKeyBytes(queueName, sqsRestoreGeneration, rec.MessageID)` at line 268. M5-3 threads a `partition *uint32` parameter through `addMessage`: when non-nil, it calls a new `sqsPartitionedMsgDataKeyBytes(queueName, *partition, sqsRestoreGeneration, rec.MessageID)`; when nil, the existing classic constructor. Adding the parameter (rather than a peer `addPartitionedMessage`) keeps the side-record dispatch in lockstep — `encodeMessageSideRecords` runs on the same record and needs the same partition info. Claude v3 noted that the doc previously left this implicit. +4. **Per-message dispatch.** `addMessage` (`encode_sqs.go:241`) currently calls `sqsMsgDataKeyBytes(queueName, sqsRestoreGeneration, rec.MessageID)` at line 268. M5-3 threads a `partition *uint32` parameter through `addMessage`: when non-nil, it calls a new `sqsPartitionedMsgDataKeyBytes(queueName, *partition, sqsRestoreGeneration, rec.MessageID)`; when nil, the existing classic constructor. Adding the parameter (rather than a peer `addPartitionedMessage`) keeps the side-record dispatch in lockstep — `addSideRecords` runs on the same record and needs the same partition info. Claude v3 noted that the doc previously left this implicit. Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): @@ -100,7 +100,7 @@ Plus three additions to `internal/backup/encode_sqs_side.go` (the M5-2 file): 2. Partitioned `byage` constructor → `!sqs|msg|byage|p|...`. 3. Partitioned `dedup` constructor → `!sqs|msg|dedup|p|...`. Note the partitioned shape adds a `` segment before `` (per `adapter/sqs_keys.go:sqsPartitionedMsgDedupKey` line N+50ish); the classic shape has only ``. This means `message_group_id` is now load-bearing for dedup-row construction on FIFO partitioned queues — but the existing `messages.jsonl` already carries it as `message_group_id`. -`encodeMessageSideRecords` (the per-message side-record dispatcher in `encode_sqs_side.go`) gains the same `partition *uint32` parameter as `addMessage` so the partitioned constructors can be selected per message. Branching by `partition != nil` (rather than re-reading `meta.PartitionCount`) keeps the call site one decision deep and makes mixed-mode bugs impossible — a classic-mode dump can never accidentally call a partitioned constructor because `rec.Partition` is enforced nil for `PartitionCount == 1` (the new fail-closed gate in §"Validation invariants" forbids any other shape). Claude v3 noted that v3 didn't spell this out. +`addSideRecords` (the per-message side-record dispatcher in `encode_sqs_side.go`) gains the same `partition *uint32` parameter as `addMessage` so the partitioned constructors can be selected per message. Branching by `partition != nil` (rather than re-reading `meta.PartitionCount`) keeps the call site one decision deep and makes mixed-mode bugs impossible — a classic-mode dump can never accidentally call a partitioned constructor because `rec.Partition` is enforced nil for `PartitionCount == 1` (the new fail-closed gate in §"Validation invariants" forbids any other shape). Claude v3 noted that v3 didn't spell this out. ## Validation invariants (fail-closed) @@ -109,6 +109,9 @@ The full decision matrix lives in the table under §"Dump-format change." The en - `meta.PartitionCount > 1` AND `rec.Partition == nil` (pre-M5-3 dump under lifted gate, or M5-3 decoder bug) → **new sentinel `ErrSQSEncodeMissingPartition`**. The operator must re-decode with an M5-3 decoder; replaying a legacy dump into a partitioned queue would silently move every message to partition 0 (codex P1 / gemini #914 v2). - `meta.PartitionCount > 1` AND `*rec.Partition >= meta.PartitionCount` → **new sentinel `ErrSQSEncodeOutOfRangePartition`** (out-of-range partition number, dump is malformed). - `meta.PartitionCount == 1` (classic) AND `rec.Partition != nil && *rec.Partition != 0` → reuses **`ErrSQSInvalidMessage`** (dump is internally inconsistent). +- `meta.PartitionCount > 1` AND `meta.FifoThroughputLimit == "perQueue"` AND `*rec.Partition != 0` → **new sentinel `ErrSQSEncodePartitionRoutingMismatch`**. The live router (`adapter/sqs_partitioning.go:71-72` in `partitionFor`) forces every group to partition 0 whenever `FifoThroughputLimit == "perQueue"`, regardless of `PartitionCount`; ReceiveMessage only scans the partition-0 lane (`adapter/sqs_keys_dispatch.go:125-126`). Accepting `*rec.Partition >= 1` for a `perQueue` queue would restore messages onto `|p|1|...` lanes the live receive fan-out never visits — silent data loss on first read. Codex P2 v914 v4 caught this gap. Pinned by `TestSQSEncodeRejectsNonzeroPartitionOnPerQueueHTFIFO`. + +The effective partition count is `1` whenever `FifoThroughputLimit == "perQueue"`, regardless of the declared `PartitionCount`. The first two gates above only fire when the effective count is `> 1` (i.e. partitioned by message group, not collapsed by per-queue throughput limiting). An impl-side helper `effectivePartitionCount(meta) uint32` is therefore preferable to inlining the perQueue check at every gate. Per-partition-record-count integrity (detecting "all messages in partition 0, none in partition 1..N-1") cannot be checked from the dump alone — the live partition assignment is `partitionFor(messageGroupID) → uint32` and the encoder doesn't run that hash. The self-test loop catches it because a re-decode would write the messages into the same partitions the encoder used — but only if the encoder respects the per-message partition, which is what the new fail-closed gates above ensure. @@ -127,7 +130,7 @@ M5-2's "full reconstruction" gate applies unchanged to M5-3 partitioned queues. ``` internal/backup/sqs.go # sqsMessageRecord +Partition *uint32; parseSQSMessageDataKey + parseSQSPartitionedQueueAndTrailer new (partition uint32) return values; HandleMessageData wires rec.Partition = &partition only when isPartitioned; parseSQSGenericKey wrapper discards the new return (claude v3 v914) internal/backup/encode_sqs.go # drop ErrSQSEncodeUnsupportedPartitioned; addMessage takes partition *uint32 + branches between sqsMsgDataKeyBytes and the new sqsPartitionedMsgDataKeyBytes -internal/backup/encode_sqs_side.go # encodeMessageSideRecords takes partition *uint32; add partitioned vis/byage/dedup constructors + emit +internal/backup/encode_sqs_side.go # addSideRecords takes partition *uint32; add partitioned vis/byage/dedup constructors + emit internal/backup/encode_sqs_test.go # round-trip partitioned-FIFO fixture (2 partitions × 3 messages) internal/backup/encode_sqs_side_test.go # cross-check partitioned vis/byage/dedup vs live constructors internal/backup/sqs_test.go # decoder round-trip with partition field populated @@ -146,6 +149,7 @@ The slice ships as a single PR — the decoder format change and encoder partiti | `TestSQSEncodeRejectsMissingPartitionOnPartitionedQueue` | `partition_count=2`, message with `Partition == nil` (legacy dump shape) → `ErrSQSEncodeMissingPartition` | | `TestSQSEncodeRejectsOutOfRangePartition` | message with `*Partition >= meta.PartitionCount` → `ErrSQSEncodeOutOfRangePartition` | | `TestSQSEncodeRejectsNonZeroPartitionOnClassicQueue` | `PartitionCount=1` but message has `Partition=2` → `ErrSQSInvalidMessage` | +| `TestSQSEncodeRejectsNonzeroPartitionOnPerQueueHTFIFO` | `PartitionCount=2` + `FifoThroughputLimit="perQueue"` + message has `*Partition=1` → `ErrSQSEncodePartitionRoutingMismatch` (codex P2 v914 v4) | | `TestSQSEncodeLegacyDumpsWithoutPartitionStillRoundTrip` | a pre-M5-3 `messages.jsonl` with no `partition` field round-trips through M5-3 encoder unchanged | | `TestSQSEncodePartitionedSideRecordsByteCrossCheckLiveAdapter` | M5-2-style cross-check: partitioned `vis|p|` / `byage|p|` / `dedup|p|` bytes equal `sqsPartitionedMsg{...}Key(...)` | From 07ee771ae558195a825e0b4ff1a6077e87bfd3bd Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:49:21 +0900 Subject: [PATCH 6/8] docs: M5-3 v6 - effectivePartitionCount naming collision + key-shape distinction (claude v914 v5) Two impl-PR notes from claude's v5 review, both low-severity but worth pinning in the doc so the impl PR doesn't trip on them: 1. effectivePartitionCount naming collision: the adapter already has an identically-named function operating on *adapter.sqsQueueMeta (sqs_keys_dispatch.go, sqs_admin_peek.go callers). The backup package cannot import it (M3b-3 circular-dep pattern, same as sqsFifoDedupWindowMillis mirrored at encode_sqs_side.go:15). v6 spells out: define a NEW copy in internal/backup/ operating on sqsQueueMetaPublic, and mirror the htfifoThroughputPerQueue constant (= "perQueue") locally. 2. Key-shape vs validation distinction: 'Branch on PartitionCount' in Encoder lift uses raw meta.PartitionCount > 1 for key shape, NOT effectivePartitionCount. perQueue queues with PartitionCount=2 still use partitioned-shape keys (|p| prefix, partition=0) because that's what the live adapter writes. effectivePartitionCount controls only validation gates and ReceiveMessage fan-out. v6 adds an explicit note so an impl author doesn't conflate them and accidentally restore perQueue dumps under classic keys. --- .../2026_06_03_proposed_sqs_partitioned_fifo_encoder.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index 3683ea0b..4498da62 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -113,6 +113,10 @@ The full decision matrix lives in the table under §"Dump-format change." The en The effective partition count is `1` whenever `FifoThroughputLimit == "perQueue"`, regardless of the declared `PartitionCount`. The first two gates above only fire when the effective count is `> 1` (i.e. partitioned by message group, not collapsed by per-queue throughput limiting). An impl-side helper `effectivePartitionCount(meta) uint32` is therefore preferable to inlining the perQueue check at every gate. +**Implementation note — define a NEW copy in `internal/backup/`.** An identically-named function already exists in `adapter/sqs_keys_dispatch.go` (operating on `*adapter.sqsQueueMeta`, the unexported live struct). The backup package MUST NOT import it — `internal/backup/` cannot pull from `adapter/` (M3b-3 circular-dependency pattern; same reason `sqsFifoDedupWindowMillis` is mirrored in `encode_sqs_side.go:15` rather than imported). The new copy operates on `sqsQueueMetaPublic` (the public/dump struct, `encode_sqs.go:71-87`). The `"perQueue"` string constant likewise must be mirrored — the adapter's `htfifoThroughputPerQueue = "perQueue"` is unexported and unimportable. Claude v914 v5 caught this naming-collision risk. + +**Key-shape vs. validation distinction.** "Branch on `PartitionCount`" in §"Encoder lift" deliberately uses the raw `meta.PartitionCount > 1` predicate, not `effectivePartitionCount`. A `perQueue` queue with `PartitionCount=2` still uses partitioned-shape keys (`|p|` prefix, all messages with partition=0) because the live adapter writes them that way. `effectivePartitionCount` controls only (a) the per-message validation gates above and (b) the ReceiveMessage scan fan-out — it does NOT control encoder key construction. Restoring a `perQueue` dump under classic keys would silently flip the queue's wire shape on read. + Per-partition-record-count integrity (detecting "all messages in partition 0, none in partition 1..N-1") cannot be checked from the dump alone — the live partition assignment is `partitionFor(messageGroupID) → uint32` and the encoder doesn't run that hash. The self-test loop catches it because a re-decode would write the messages into the same partitions the encoder used — but only if the encoder respects the per-message partition, which is what the new fail-closed gates above ensure. ## Decision gate: full reconstruction vs lazy rebuild (carry-over from M5-2) From 0c7782fc0cf029d133ae15ff36903b4935ecd960 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:53:27 +0900 Subject: [PATCH 7/8] docs: M5-3 v7 - correct sqsQueueMetaPublic file path (claude v914 v6) claude v6 caught that v6's reference to sqsQueueMetaPublic at encode_sqs.go:71-87 was wrong: - sqsQueueMetaPublic actually lives at internal/backup/sqs.go:148 - encode_sqs.go:66 defines a DIFFERENT struct, sqsStoredQueueMeta, used for the !sqs|queue|meta| record value (live-adapter shape), not the dump-side public projection. An impl PR author searching encode_sqs.go:71-87 would have found the wrong struct. v7 corrects the file path and adds a one-line note distinguishing the two structs. --- docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index 4498da62..41ed5a07 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -113,7 +113,7 @@ The full decision matrix lives in the table under §"Dump-format change." The en The effective partition count is `1` whenever `FifoThroughputLimit == "perQueue"`, regardless of the declared `PartitionCount`. The first two gates above only fire when the effective count is `> 1` (i.e. partitioned by message group, not collapsed by per-queue throughput limiting). An impl-side helper `effectivePartitionCount(meta) uint32` is therefore preferable to inlining the perQueue check at every gate. -**Implementation note — define a NEW copy in `internal/backup/`.** An identically-named function already exists in `adapter/sqs_keys_dispatch.go` (operating on `*adapter.sqsQueueMeta`, the unexported live struct). The backup package MUST NOT import it — `internal/backup/` cannot pull from `adapter/` (M3b-3 circular-dependency pattern; same reason `sqsFifoDedupWindowMillis` is mirrored in `encode_sqs_side.go:15` rather than imported). The new copy operates on `sqsQueueMetaPublic` (the public/dump struct, `encode_sqs.go:71-87`). The `"perQueue"` string constant likewise must be mirrored — the adapter's `htfifoThroughputPerQueue = "perQueue"` is unexported and unimportable. Claude v914 v5 caught this naming-collision risk. +**Implementation note — define a NEW copy in `internal/backup/`.** An identically-named function already exists in `adapter/sqs_keys_dispatch.go` (operating on `*adapter.sqsQueueMeta`, the unexported live struct). The backup package MUST NOT import it — `internal/backup/` cannot pull from `adapter/` (M3b-3 circular-dependency pattern; same reason `sqsFifoDedupWindowMillis` is mirrored in `encode_sqs_side.go:15` rather than imported). The new copy operates on `sqsQueueMetaPublic` (the public/dump struct, `internal/backup/sqs.go:148`). Note: `encode_sqs.go:66` defines a different struct, `sqsStoredQueueMeta`, used for the `!sqs|queue|meta|` value — not the dump-side struct (claude v914 v6 caught the earlier file-path error). The `"perQueue"` string constant likewise must be mirrored — the adapter's `htfifoThroughputPerQueue = "perQueue"` is unexported and unimportable. Claude v914 v5 caught this naming-collision risk. **Key-shape vs. validation distinction.** "Branch on `PartitionCount`" in §"Encoder lift" deliberately uses the raw `meta.PartitionCount > 1` predicate, not `effectivePartitionCount`. A `perQueue` queue with `PartitionCount=2` still uses partitioned-shape keys (`|p|` prefix, all messages with partition=0) because the live adapter writes them that way. `effectivePartitionCount` controls only (a) the per-message validation gates above and (b) the ReceiveMessage scan fan-out — it does NOT control encoder key construction. Restoring a `perQueue` dump under classic keys would silently flip the queue's wire shape on read. From ed24e2aabf8d77d53f4358242c636f5c07442dfa Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 3 Jun 2026 21:58:59 +0900 Subject: [PATCH 8/8] docs: M5-3 v8 - revert effectivePartitionCount in validation gates (codex P2 v914 v7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v6 introduced a subtle data-loss bug by saying the missing-partition and out-of-range gates should use effectivePartitionCount. Codex P2 v914 v7 caught it: For perQueue queues with PartitionCount=2: - effectivePartitionCount collapses to 1 - v6 said: missing-partition gate doesn't fire (since effective=1) - So a perQueue dump with rec.Partition == nil would slip past validation - addMessage branches on partition != nil; nil -> classic sqsMsgDataKeyBytes - But the LIVE key dispatch selects the partitioned keyspace from raw meta.PartitionCount > 1 (sqs_keys_dispatch.go) - Result: restored messages get classic keys for a queue whose readers scan only the partitioned keyspace -> silent data loss v8 reverts: ALL four gates use raw meta.PartitionCount > 1 as the partitioned-queue predicate. effectivePartitionCount stays useful for diagnostics and the ReceiveMessage scan fan-out, but MUST NOT appear in any encoder-side validation gate predicate. Updated the 'Key-shape vs validation distinction' section to unify both choices (key shape AND validation) on raw PartitionCount. Caller audit: this is a semantic change in the gate predicate from 'effective' back to 'raw'. The only callers of these predicates are the encoder's per-message loop (single site), which is what we're fixing. The effectivePartitionCount helper itself is unchanged — it still mirrors adapter/sqs_keys_dispatch.go's helper for the ReceiveMessage fan-out cross-check. --- .../2026_06_03_proposed_sqs_partitioned_fifo_encoder.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md index 41ed5a07..1986bc94 100644 --- a/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md +++ b/docs/design/2026_06_03_proposed_sqs_partitioned_fifo_encoder.md @@ -111,11 +111,11 @@ The full decision matrix lives in the table under §"Dump-format change." The en - `meta.PartitionCount == 1` (classic) AND `rec.Partition != nil && *rec.Partition != 0` → reuses **`ErrSQSInvalidMessage`** (dump is internally inconsistent). - `meta.PartitionCount > 1` AND `meta.FifoThroughputLimit == "perQueue"` AND `*rec.Partition != 0` → **new sentinel `ErrSQSEncodePartitionRoutingMismatch`**. The live router (`adapter/sqs_partitioning.go:71-72` in `partitionFor`) forces every group to partition 0 whenever `FifoThroughputLimit == "perQueue"`, regardless of `PartitionCount`; ReceiveMessage only scans the partition-0 lane (`adapter/sqs_keys_dispatch.go:125-126`). Accepting `*rec.Partition >= 1` for a `perQueue` queue would restore messages onto `|p|1|...` lanes the live receive fan-out never visits — silent data loss on first read. Codex P2 v914 v4 caught this gap. Pinned by `TestSQSEncodeRejectsNonzeroPartitionOnPerQueueHTFIFO`. -The effective partition count is `1` whenever `FifoThroughputLimit == "perQueue"`, regardless of the declared `PartitionCount`. The first two gates above only fire when the effective count is `> 1` (i.e. partitioned by message group, not collapsed by per-queue throughput limiting). An impl-side helper `effectivePartitionCount(meta) uint32` is therefore preferable to inlining the perQueue check at every gate. +**All four gates above use raw `meta.PartitionCount > 1` as the partitioned-queue predicate.** Codex P2 v914 v7 caught a subtle but data-loss-prone error in v6, which proposed switching the missing-partition / out-of-range gates to `effectivePartitionCount`: that would loosen the missing-partition gate so a perQueue dump with `Partition == nil` would slip past validation, then `addMessage`'s `partition != nil` branch would emit classic-shape `!sqs|msg|data|` keys for a queue whose live readers scan only the partitioned `!sqs|msg|data|p|0|` keyspace (key shape is selected from raw `PartitionCount > 1` in `adapter/sqs_keys_dispatch.go`). Result: restored messages would be invisible. The `effectivePartitionCount(meta) uint32` helper is still useful for diagnostics and for the ReceiveMessage scan fan-out, but it MUST NOT replace the raw predicate in encoder validation gates. **Implementation note — define a NEW copy in `internal/backup/`.** An identically-named function already exists in `adapter/sqs_keys_dispatch.go` (operating on `*adapter.sqsQueueMeta`, the unexported live struct). The backup package MUST NOT import it — `internal/backup/` cannot pull from `adapter/` (M3b-3 circular-dependency pattern; same reason `sqsFifoDedupWindowMillis` is mirrored in `encode_sqs_side.go:15` rather than imported). The new copy operates on `sqsQueueMetaPublic` (the public/dump struct, `internal/backup/sqs.go:148`). Note: `encode_sqs.go:66` defines a different struct, `sqsStoredQueueMeta`, used for the `!sqs|queue|meta|` value — not the dump-side struct (claude v914 v6 caught the earlier file-path error). The `"perQueue"` string constant likewise must be mirrored — the adapter's `htfifoThroughputPerQueue = "perQueue"` is unexported and unimportable. Claude v914 v5 caught this naming-collision risk. -**Key-shape vs. validation distinction.** "Branch on `PartitionCount`" in §"Encoder lift" deliberately uses the raw `meta.PartitionCount > 1` predicate, not `effectivePartitionCount`. A `perQueue` queue with `PartitionCount=2` still uses partitioned-shape keys (`|p|` prefix, all messages with partition=0) because the live adapter writes them that way. `effectivePartitionCount` controls only (a) the per-message validation gates above and (b) the ReceiveMessage scan fan-out — it does NOT control encoder key construction. Restoring a `perQueue` dump under classic keys would silently flip the queue's wire shape on read. +**Key-shape vs. validation distinction.** Both the "Branch on `PartitionCount`" key-shape choice in §"Encoder lift" AND the validation gates above use the raw `meta.PartitionCount > 1` predicate, never `effectivePartitionCount`. A `perQueue` queue with `PartitionCount=2` writes partitioned-shape keys (`|p|` prefix, all messages with partition=0) — the live adapter does this because `meta.PartitionCount > 1` selects the partitioned keyspace at dispatch, and `partitionFor` separately collapses every group to partition 0. Restoring such a dump under classic keys would silently flip the queue's wire shape, and accepting a `Partition == nil` message would similarly cause `addMessage` to emit classic keys via the `partition != nil` dispatch — making restored rows invisible on first read. Per-partition-record-count integrity (detecting "all messages in partition 0, none in partition 1..N-1") cannot be checked from the dump alone — the live partition assignment is `partitionFor(messageGroupID) → uint32` and the encoder doesn't run that hash. The self-test loop catches it because a re-decode would write the messages into the same partitions the encoder used — but only if the encoder respects the per-message partition, which is what the new fail-closed gates above ensure.