Partition keys (1A) + producer idempotency (1B): spec, brief, draft implementation

[NikolayS]

What's here

Design layer (converged through 3 internal review rounds + Max + Fabrizio + two stronger-model refinement passes):

• blueprints/partition-keys/SPEC.md v0.7 — ordered per-key consumption via N slot subscriptions; G1/G2 corrected (session claim is load-bearing, not "pure liveness"); mechanism/policy seam; member/heartbeat table rejected (D10); tick-gated online-resize protocol as sanctioned future path (D9, draft); connection-pooler caveat (only the claim connection needs session mode).
• blueprints/idempotency/SPEC.md — producer TTL dedup as a separate send-layer feature.
• Brief deployed at https://pgque.dev/briefs/partition-keys.html with full version history.

User stories — US-12.1–12.7 (lifecycle events: keyed send, per-key order, cross-key parallelism, single processor, claim/crash-recovery, observability, enforced N) and US-13.1–13.5 (migrations: TTL dedup, effect-scoped keys, expiry, GC, mutual-exclusion recipe) in both SPECs, the brief, and executable acceptance tests.

Draft implementation (red/green TDD, engine untouched):

• sql/pgque-api/partition_keys.sql — 4-arg send, subscribe_slot/unsubscribe_slot (persisted enforced N), receive_partitioned (SECURITY DEFINER co-ownership over the admin-only get_batch_cursor extra_where hook), ack_partitioned, slot_lock_key/claim_slot/release_slot, partition_slot_status view.
• sql/pgque-api/send_idem.sql — atomic single-statement TTL claim, dedup returns the original event_id, maint_idem() GC self-registered via queue_extra_maint (zero maint.sql edits).
• Wired into build/transform.sh; sql/pgque.sql + pgque-tle.sql rebuilt.

Reproduction & benchmarks (earlier commits): TS/bun repro of both workloads incl. the version-suppression hazard guardrail; bloat-under-backlog + throughput bench vs a pg-boss-style mutable job table.

Verification (fresh PostgreSQL 18.3)

1. install (\i sql/pgque.sql)                      OK
2. tests/run_all.sql                               ALL TESTS PASSED (173 PASS)
3. tests/acceptance/run_acceptance.sql (US-1..13)  ALL ACCEPTANCE TESTS PASSED
4. tests/two_session_slot_claim.sh                 PASS (claim exclusivity across
   real sessions; dead session's slot instantly reclaimable; owner_pid visible)

Repro commands:

psql -d pgque_test -f sql/pgque.sql
psql -d pgque_test -f tests/run_all.sql
psql -d pgque_test -f tests/acceptance/run_acceptance.sql
PGQUE_TEST_DSN=postgresql://...:5499/pgque_test tests/two_session_slot_claim.sh

Known follow-ups before leaving draft

• T-security non-superuser-installer variant (co-ownership currently exercised as superuser only)
• CI across PG 14–17 (locally verified on 18 only)
• Online resize (D9) is spec-only, marked draft-pending-its-own-review — deliberately NOT implemented
• pause policy (Phase 2) blocked on O1 — deliberately NOT implemented

🤖 Generated with Claude Code

https://claude.ai/code/session_01HYTw9GXVXJvectNqZ2fqsq

[NikolayS]

Partition-keys benchmark — v0.8 lease model at the target scale (SPEC R2/R7, S4)

Question this answers: does the N-slot + batch-lease design sustain a high-volume multi-tenant profile — >400M events/day, ordered per tenant — and what do read amplification (R2) and rotation pinning (R7) cost in practice?

Setup

• Hardware: Hetzner CCX43 — 16 dedicated vCPU (AMD), 64 GiB RAM, local NVMe, Ubuntu 24.04.
• PostgreSQL 18 (PGDG): shared_buffers=16GB, synchronous_commit=off, wal_level=minimal, max_wal_size=16GB, checkpoint_timeout=15min, wal_compression=on, jit=off, pg_stat_statements preloaded.
• pgque: this PR's sql/pgque.sql at v0.8 (batch-lease slot ownership).
• Workload: pgbench producer at rate-controlled 5,000 ev/s (= 432M events/day), ~200-byte JSON payloads, partition key tenant-<n> drawn random_zipfian(1, 2000, 1.1) — hot-head tenant skew. Consumers: one bun worker per slot driving the real API — claim_slot (30 s TTL) → sticky receive_partitioned (batch 500) → ack_partitioned — i.e. the documented lease loop, no shortcuts. Ticker every 250 ms, maint() every 60 s, queue rotation period 30 s.
• Harness: benchmark/partition-keys/ in this PR (samplers: per-slot lag 5 s, sys 10 s, pg_stat_statements 10 s, per-table bloat 30 s).

Headline

Phase	N	Producer ev/s	Consume ev/s	Pending p99 (events)	Buffers/event	CPU peak	Peak dead tup (meta)	Event tbl GiB end
steady-16 (30 min)	16	5,000	4,999	1,320	4.39	26%	7,586	3.57
steady-32 (30 min)	32	5,001	5,000	1,318	7.02	31%	9,502	7.19
stalled-16 (15 min, slot 7 SIGSTOP 8 min)	16	4,997	4,996	1,665,413	2.97	30%	8,912	5.38

Steady state: the design holds the target with a wide margin. ~9.0M events produced and consumed per 30-minute phase; per-slot pending p50 0, p99 ~1,320 events (≈ 0.26 s of production); zero buffer reads (fully cache-resident window); CPU peak 26–31% on 16 cores. Doubling N from 16→32 at the same rate changed steady-state lag not at all.

Read amplification (R2)

N	buffers/event	32/16 ratio
16	4.39	—
32	7.02	1.60× (ideal-linear would be 2×)

Every slot scans the full stream and filters server-side, so buffers touched per produced event grow with N — observed sublinear (1.60× for 2× slots), all from shared_buffers at this scale. Practical read: at 5k ev/s, going from 16 to 32 slots costs ~5 CPU points. R2 is real, measurable, and affordable at this profile; it argues for keeping N at the parallelism you need, as the SPEC says.

Rotation pinning (R7) — demonstrated, quantified

Slot 7's worker SIGSTOPped for 8 minutes mid-phase:

• Its lag grew at 2,663 events/s (its zipfian share of the stream), peaking at 2.39M pending.
• Rotation floor pinned: event tables stopped dropping; event-table bytes grew 3.62 → 5.38 GiB during the stall and resumed dropping after.
• Other 15 slots: completely unaffected (p50 pending stayed 0).
• Catch-up did not complete within the remaining 5 minutes after resume — a stalled slot's backlog drains at (slot consume rate − slot arrival rate), so recovery is not instant at this volume.

Two operational findings worth pinning in the docs:

1. The freed slot was not taken over. The stopped worker's lease expired after its 30 s TTL, but with M=N sticky workers each bound to its own slot, nobody polled slot 7 — it sat ownerless for ~15 min. This is exactly SPEC §15's "every slot must be polled" requirement showing its teeth: client claim loops should cycle idle/free slots (or run M>N candidates), not only stick to their own.
2. Stale subscriptions are not inert (found by accident): a 4-slot consumer left over from a smoke run silently pinned rotation for the whole queue while every live metric looked healthy. The per-slot lag alert on partition_slot_status is mandatory, and unsubscribe_slot on teardown is not optional. (The harness now force-drops the bench queue at setup for this reason.)

Dead tuples & bloat

• Event tables: 0 dead tuples throughout all phases. Append-only + rotation-by-drop holds at 432M/day equivalent — the core pgque thesis.
• partition_slot (the v0.8 lease, 2 UPDATEs/batch): peak 557–704 dead tuples, steady ~96 KiB across all phases. HOT keeps lease churn negligible — the "no per-interval heartbeat churn" claim (D10/D11) is confirmed empirically.
• subscription / tick metadata churn peaked at ~9.5k / ~7.2k dead tuples and returned to ~0 — bounded rotation-metadata behavior.
• Total DB footprint stayed = live event window (3.6–7.2 GiB, proportional to rotation retention), no growth beyond it except during the deliberate R7 stall.

Caveats

• Handler is trivial (count + ack): this measures the queue's capacity, not handler capacity; real deployments add their own per-event work.
• Producer and consumers ran on the DB host over localhost — no network latency in the loop.
• 30-minute steady phases; no multi-day soak here (the 2-hour held-xmin soak methodology exists separately in benchmark/).
• Catch-up-after-stall was measured but not to completion; drain-time modeling for a lagged slot is worth a follow-up note.

Bottom line for the >400M/day question: steady state sustains it at ~26–31% peak CPU on 16 dedicated cores with sub-second per-slot lag and zero event-table bloat, at both N=16 and N=32. The two things that need operational discipline are exactly the two the SPEC already names: keep every slot polled (R7 + §15) and alert on per-slot lag.

Raw data: /tmp/bench/pk/ CSVs (per-slot lag 5 s, pgss 10 s, bloat 30 s, sys 10 s) — archived before VM teardown; harness committed in benchmark/partition-keys/.

[NikolayS]

Forward-looking: does partition-keys shard? (roadmap note — out of scope for this PR)

Recording this because it came up while reviewing the design, and because it's evidence the abstraction is cut at the right joint. Not proposed for this PR — this is Phase-N thinking.

It composes cleanly, and here's why

Every guarantee this feature makes is per-key: G1 (order within a key), G2 (single processor per key), retry affinity, and the send_idem dedup window. There is deliberately no cross-key guarantee — no global order, no cross-key transaction. That is exactly the contract shape that lets Kafka/Kinesis shard: if the unit of guarantee is the key, splitting keys across nodes loses nothing, because nothing was ever promised between keys.

So the sharded model is the obvious one — key routes to a shard, shard is a full independent pgque install:

producer → shard = hash(key) % S  →  shard s = its own pgque (own log, ticker, N slots, DLQ)
                                      then inside the shard: slot = hash(key) % N

Two-level hashing: key picks the shard (level 1), then the existing (hashtextextended(key,0) % N + N) % N picks the slot (level 2). Same key → one shard → one slot → G1/G2 hold globally by composition, with zero engine changes. send_idem survives too: dedup stays per-shard, but a given (key, idem_key) always routes to the same shard, so per-shard is sufficient.

The one trap worth writing down now

If both levels use the same hash, shard count S and slot count N must not share factors. With S=8, N=4: every key in shard s satisfies hash % 8 = s, which fully determines hash % 4 — so all keys in a shard collapse onto one slot and the other three sit empty (and by R7, stall-if-unpolled). Fix is trivial but must be specified: different seed per level — hashtextextended(key, 1) for the shard, seed 0 for the slot (the seed arg exists for exactly this) — rather than relying on coprime S/N.

The three routers

• PgDog — workable today. A sharding pooler over independent Postgres nodes — precisely the "S independent installs" shape. pgque.send($1,…) doesn't look shardable to its query inspector, so the robust path is client-side routing: a ShardedProducer holding S pools that computes the shard itself (~50 lines/client). The v0.8 lease's pooler-friendliness carries over per-shard unchanged.
• Citus — poor fit, don't. The engine is node-local by construction: batch windows come from pg_current_snapshot() (no distributed snapshot in Citus), rotation creates/drops event tables, tick machinery assumes local xid visibility. Distributing pgque.event_* means rewriting the sacred engine (Design Rule 2). The only Citus shape that works is "independent pgque per worker node" — i.e. the PgDog shape with worse ergonomics.
• Multigres — natural future home. Vitess-for-Postgres: routing over full Postgres shards, each running unmodified pgque, with first-class shard split/merge. Notably our D9 online-resize protocol (epoch-gated drain-then-cutover, modeled on Kinesis parent-shard drain) is the same protocol a shard split needs — seal, drain the moving key range, cut over. The vocabulary already exists in the spec.

The strongest argument

If the app's own data is sharded by the same key (the multi-tenant profile in the benchmark almost certainly is), app rows and their queue land on the same shard — preserving the transactional-outbox property: insert app row + pgque.send in one local transaction. Kafka fundamentally cannot give you that. The honest headline for a future sharded-pgque brief: Kafka-style horizontal scale-out, keeping same-shard transactional produce.

Relation to the benchmark just posted

Sharding is orthogonal to the read-amplification result above, and complementary: read-amp is the ~N× cost of the slot filter within one install (measured 4.39 → 7.02 buffers/event for N=16 → 32). Sharding scales write/storage/CPU horizontally across nodes; slots give per-key parallelism within a shard. At the volume benchmarked (5k ev/s ≈ 432M/day) a single 16-core node sits at ~30% CPU, so sharding isn't needed here — it's the lever for the 10× beyond, or for spreading storage. The single-reader dispatch idea (R6) is the other answer to read-amp and is independent of sharding.

What it would cost pgque

Almost nothing in core — that's the point:

1. A short blueprints/sharding/ brief: hash + seed discipline + an S registry.
2. Client-lib ShardedProducer/ShardedConsumer — routing + the existing claim loop per shard. Policy side of the mechanism/policy seam (D8), so it belongs in clients.
3. Docs on what does not shard: trigger-sourced queues, keyless queues, any hope of global order, and cross-shard DLQ/observability (aggregate in the CLI).
4. Later, if Multigres matures: map shard-split onto the D9 drain protocol.

[NikolayS]

samorev Code Review Report

• PR: Partition keys (1A) + producer idempotency (1B): spec, brief, draft implementation #295 - Partition keys (1A) + producer idempotency (1B): spec, brief, draft implementation
• Author: @NikolayS
• AI-Assisted: Unknown

Pipeline	Coverage
PASS	Not reported

---

BLOCKING ISSUES (1)

HIGH MR/PR state - Review target is draft

The review target is still marked as draft.
Fix: Mark it ready for review before merge.

---

Summary

Area	Findings	Potential	Filtered
CI/Pipeline	0	0	0
Security	0	0	0
Bugs	0	0	0
Tests	0	0	0
Guidelines	0	0	0
Docs	0	0	0
Metadata	1	0	0

Note:

• Findings: High-confidence issues (8-10/10) - blocking or non-blocking per severity
• Potential: Medium-confidence issues (4-7/10) - review manually
• Filtered: Low-confidence issues (0-3/10) - excluded as likely false positives

Review metadata

provider=github
kind=pr
project=NikolayS/PgQue
number=295
target=github:NikolayS/PgQue#295
state=OPEN
draft=true
diff_lines=1
diff_added=0
diff_removed=0
diff_bytes=71
comments_count=2
commits_count=27
ci_status=success
ci_summary=total=17 success=17 failure=0 pending=0 other=0
prompt=.claude/commands/review-mr.md
blocking=false
posted_by=gh
no_comment=false
live_posting=posted

---

samorev-assisted review (AI analysis by Tanya301/samorev)

[NikolayS]

Deep pre-merge review — PR #295 (partition keys 1A + producer idempotency 1B)

Reviewed the rebased head d0e0d93. Focus was the hand-written code
(partition_keys.sql, send_idem.sql, the receive.sql guards + shared
_nack_batch_event), build wiring, tests, and security. Verdict up front:
merge-ready for the 0.3.0-alpha1 cut — no blocking findings. Details, then
a short list of minors/nits.

What I verified (all green)

• Build reproduces. Fresh worktree, git submodule update --init, bash build/transform.sh → empty git diff. partition_keys.sql +
  send_idem.sql are wired into DEFAULT_API_FILES after receive.sql, and
  the generated sql/pgque.sql / sql/pgque-tle.sql match.
• Tests pass on PG 18 (local): tests/test_partition_keys.sql (all US-12.x
  • fencing + retry-affinity + raw-slot guards), tests/test_send_idem.sql (all
    US-13.x incl. the dblink I2 race and I3 atomic rollback), and
    tests/two_session_slot_claim.sh (cross-backend lease exclusivity + TTL
    takeover + epoch bump).
• SECURITY DEFINER hygiene: every new definer function pins set search_path = pgque, pg_catalog. Confirmed on all of send/subscribe_slot/
  unsubscribe_slot/claim_slot/release_slot/receive_partitioned/
  ack_partitioned/nack_partitioned/_partition_n/_touch_lease/_slot_name
  and both send_idem overloads + maint_idem.
• No subtransactions in hot paths. No begin…exception in the ticker/insert/
  receive/ack/nack/send_idem hot paths. send_idem's claim+append is a single
  atomic upsert (Key Design Rule 4), and the exception blocks that exist are in
  tests and in dlq_replay_all (not a hot path).
• Engine untouched. The only non-generated, non-API change is
  sql/pgque-additions/dlq.sql (see nit 5). get_batch_cursor/4,
  next_batch, finish_batch, batch_event_sql, rotation, tick are unmodified;
  the co-ownership path into the admin-only get_batch_cursor(4) is used exactly
  as the spec (§6) describes.
• No injection. The hash-filter format() interpolates only the validated
  integers n/slot; queue/consumer names are never spliced into the trusted-SQL
  sink. The only other dynamic SQL is the cursor name 'pgque_part_'||batch_id
  (bigint, quote_ident'd on close).
• Concurrency reasoning holds. _touch_lease/claim_slot serialize on the
  partition_slot row via for update, so fence-vs-takeover is race-safe; the
  zombie's post-takeover ack raises on the owner mismatch before it can
  double-finish (and even absent the fence, sub_batch is already null →
  ack_partitioned returns 0). send_idem's upsert is safe under the concurrent
  fresh/expired/rolled-back-winner cases (walked each; the ON CONFLICT WHERE
  expires_at <= now() + PK serialization behaves as documented).

Findings

[minor] # reserved by the raw-slot guard but not enforced at plain consumer
creation — sql/pgque-api/receive.sql:48 (and :99, :194).
The guards reject any consumer name containing #, but pgque.subscribe /
register_consumer do not reject # (only cooperative names reject .,
pgque.sql:6026). So a legitimately-created plain consumer whose name contains
# becomes permanently unusable through receive/ack/nack. Reproduced:

select pgque.subscribe('hashq', 'team#1');   -- succeeds
-- after a send+tick:
perform * from pgque.receive('hashq','team#1',10);
-- ERROR: consumer team#1 is a partition slot consumer; use pgque.receive_partitioned()

Not a security hole and #-in-names is unusual, but it's an unenforced global
reservation. Fix options: (a) reject # in register_consumer/subscribe too
(make the reservation real and symmetric), or (b) tighten the guard to match the
actual slot grammar (… ~ '#\d+/\d+$') instead of position('#' …) > 0.

[minor] i_max truncation silently skips a slot's surplus on ack —
partition_keys.sql:522 / ack_partitioned.
receive_partitioned fetches up to i_max filtered rows from the batch
cursor; ack_partitioned then finish_batches the whole tick window. If a
single slot's hash-class slice in one window exceeds i_max (default 100), the
surplus is acked without ever being delivered. This is the same contract as plain
pgque.receive (inherited, not introduced here), but the hash filter makes the
per-slot count harder to predict than a raw batch size. Worth an explicit doc
note ("size i_max ≥ the largest per-slot slice a tick window can hold, or the
overflow is skipped on ack"), since the partitioned path is where an operator is
least likely to reason about it.

[nit] Design-note comment conflates the server fence with the epoch token —
partition_keys.sql:49-50. "Takeover bumps the epoch, fencing the zombie — its
next receive/ack raises." The server-side fence is actually the lease_owner <> i_worker string check in _touch_lease; epoch is a user-space fencing token
returned by claim_slot and is never consulted by receive/ack. Behaviour is
correct and matches the spec (§8/§15, which requires unique worker ids per live
worker), but the inline comment reads as if the epoch does the server-side
fencing. A one-line clarification would prevent a future reader from assuming
epoch is checked on the receive/ack path.

[nit] send_idem dedup can return a rotated-away event_id —
send_idem.sql:150. If the idem TTL outlives event retention, the deduped = true path returns the original event_id for an event that has already been
consumed/rotated and no longer exists in any data table. Semantically fine
("already sent"), but callers that try to dereference the returned id will miss;
one doc sentence would set expectations.

[nit] Bundled DLQ concurrency hardening — sql/pgque-additions/dlq.sql.
dlq_replay/dlq_replay_all gain for update of dl / for update … skip locked. Sound and welcome, but orthogonal to partition-keys/idempotency — make
sure it lands in the changelog under its own heading rather than the feature line.

Notes (explicitly not findings)

• Fencing model, i_max default, and read-amp/rotation-pinning (R2/R7) are honest
  and documented; the lease's process→ack exclusivity is correctly scoped to the
  pgque-api surface (raw PgQ primitives stay cooperatively trusted).
• Known open items were not re-reported (non-superuser-installer T-security
  variant, D9 online resize, Phase-2 pause, partition_slot_status
  cross-queue reader visibility, PR size).

[NikolayS]

Addressed the #-in-consumer-name lockout reported in #295 (comment). Went with option (a): pgque.subscribe() now rejects any consumer name containing the reserved # at registration time (clear error: consumer name must not contain the reserved character #), so a plain consumer can no longer be created into a state the receive/ack/nack slot guards would permanently refuse. The inherited PgQ register_consumer engine is untouched — the check lives in the API-layer wrapper. The broad position('#' ...) guards on receive/ack/nack stay as defense in depth; since no plain consumer can now carry #, they only ever fire for real slot consumers, so the team#1/2 grammar-collision concern noted for option (b) does not arise.

Red/green TDD: added a guard case in tests/test_partition_keys.sql asserting subscribe('pk_q','team#1') raises and leaves no consumer row — confirmed failing on the prior head, green after the fix. Full tests/run_all.sql and tests/acceptance/run_acceptance.sql pass against Postgres 17 in Docker. Sources under sql/pgque-api/ and the regenerated sql/pgque.sql / sql/pgque-tle.sql are committed together (bash build/transform.sh leaves a clean tree).

New head: b7244ab

[NikolayS]

Diff-size reduction pass: 12,783 → 10,068 additions (50 → 29 files).

• Benchmark harness (benchmark/partition-keys/) and the read-amp repro (blueprints/partition-keys/repro/) moved byte-identical to the follow-up draft PR feat(bench): partition-keys benchmark + read-amp repro harness #297, along with the bench .gitignore entry. Spec/brief references to blueprints/partition-keys/repro/ resolve once that PR merges (it should follow this one).
• refactor(partition-keys): the consume-path trio (receive/ack/nack_partitioned) shared a validate-N + queue lookup + lease-fence preamble and (ack/nack) an open-batch lookup; merged into _slot_guard / _slot_batch. Error precedence and messages unchanged; one queue lookup saved per consume call. Generated files rebuilt with build/transform.sh.

Verification on docker postgres:17 (fresh install from sql/pgque.sql):

• tests/run_all.sql — pass (ON_ERROR_STOP)
• tests/acceptance/run_acceptance.sql — pass
• tests/two_session_slot_claim.sh — pass
• reinstall idempotency (sql/pgque.sql re-run) — pass
• upgrade from main's install, then tests/run_all.sql — pass
• upgrade from the pre-refactor draft install — pass; stale _partition_n/_touch_lease confirmed dropped

🤖 Generated with Claude Code

[NikolayS]

samorev Code Review Report

• PR: Partition keys (1A) + producer idempotency (1B): spec, brief, draft implementation #295 - Partition keys (1A) + producer idempotency (1B): spec, brief, draft implementation
• Author: @NikolayS
• AI-Assisted: Yes

Pipeline	Coverage
PASS	Not reported

---

This report supersedes the earlier samorev review --fetch comment on this PR. That comment was a CI/draft gate check only and performed no code analysis (it hardcoded zeros in every area). The results below come from a genuine per-area review of the hand-written code (partition_keys.sql, send_idem.sql, the receive.sql guards + shared _nack_batch_event), build wiring, tests, and security, verified on PostgreSQL 18.

No blocking issues for the 0.3.0-alpha1 cut. Two non-blocking notes and two lower-confidence items below.

Review memory

• Previously flagged, now RESOLVED: the #-reserved-but-unenforced lockout (a plain consumer created with # in its name became permanently unusable through receive/ack/nack) has been fixed at the current head — pgque.subscribe/register_consumer now reject the reserved # at registration (consumer name must not contain the reserved character #). The broad position('#' …) guards remain as defense in depth. Not re-flagged.

What was verified

• Build reproduces: fresh worktree, submodule init, bash build/transform.sh → empty git diff. partition_keys.sql + send_idem.sql wired into DEFAULT_API_FILES after receive.sql; generated files match.
• Tests pass on PG 18: test_partition_keys.sql (US-12.x + fencing + retry-affinity + raw-slot guards), test_send_idem.sql (US-13.x incl. the dblink I2 race and I3 atomic rollback), and two_session_slot_claim.sh (cross-backend lease exclusivity + TTL takeover + epoch bump).
• Security definer hygiene: every new definer function pins set search_path = pgque, pg_catalog (all of send/subscribe_slot/unsubscribe_slot/claim_slot/release_slot/receive_partitioned/ack_partitioned/nack_partitioned/_partition_n/_touch_lease/_slot_name, both send_idem overloads + maint_idem).
• No subtransactions in hot paths; engine untouched. send_idem's claim+append is a single atomic upsert. get_batch_cursor/4, next_batch, finish_batch, batch_event_sql, rotation, tick unmodified.
• No injection: the hash-filter format() interpolates only validated integers; names never reach the trusted-SQL sink. Concurrency reasoning holds — _touch_lease/claim_slot serialize on the partition_slot row via for update, so fence-vs-takeover is race-safe.

---

NON-BLOCKING (2)

LOW sql/pgque-api/partition_keys.sql (ack_partitioned) - i_max truncation silently skips a slot's surplus on ack

receive_partitioned fetches up to i_max filtered rows; ack_partitioned then finish_batches the whole tick window. If one slot's hash-class slice in a window exceeds i_max (default 100), the surplus is acked without ever being delivered. Same contract as plain pgque.receive (inherited), but the hash filter makes the per-slot count harder to predict.
Suggestion: Add an explicit doc note ("size i_max ≥ the largest per-slot slice a tick window can hold, or the overflow is skipped on ack") on the partitioned path, where an operator is least likely to reason about it.

INFO sql/pgque-api/partition_keys.sql:49-50 - design-note comment conflates the server fence with the epoch token

The comment reads as if the epoch fences the zombie, but the server-side fence is the lease_owner <> i_worker check in _touch_lease; epoch is a user-space token returned by claim_slot and never consulted on the receive/ack path. Behaviour is correct and matches the spec.
Suggestion: One-line clarification so a future reader doesn't assume epoch is checked server-side.

---

POTENTIAL ISSUES (2)

LOW sql/pgque-api/send_idem.sql - dedup return contract + TTL-vs-retention semantics need a doc sentence (confidence: 5/10)

If the idem TTL outlives event retention, a deduped = true result refers to an event that may already have been consumed/rotated away. The return contract (deduped → event_id = null, nothing appended) should be stated so callers don't try to dereference an id for a since-rotated event.
Suggestion: One doc sentence setting expectations on the deduped return and the TTL/retention relationship.

INFO sql/pgque-additions/dlq.sql - bundled DLQ concurrency hardening is orthogonal to this PR (confidence: 5/10)

dlq_replay/dlq_replay_all gain for update of dl / for update … skip locked. Sound and welcome, but unrelated to partition-keys/idempotency.
Suggestion: Make sure it lands in the changelog under its own heading rather than the feature line.

---

Summary

Area	Findings	Potential	Filtered
CI/Pipeline	0	0	0
Security	0	0	0
Bugs	0	0	0
Tests	0	0	0
Guidelines	0	1	0
Docs	2	1	0
Metadata	0	0	0

Note:

• Findings: High-confidence issues (8-10/10) - blocking or non-blocking per severity
• Potential: Medium-confidence issues (4-7/10) - review manually
• Filtered: Low-confidence issues (0-3/10) - excluded as likely false positives

Known open items already tracked on this PR were not re-reported (non-superuser-installer T-security variant, D9 online resize, Phase-2 pause, partition_slot_status cross-queue reader visibility, PR size — the author has since run a diff-size reduction pass).

Review metadata

provider=github
kind=pr
project=NikolayS/pgque
number=295
target=github:NikolayS/pgque#295
state=OPEN
draft=false
diff_lines=11235
diff_added=10202
diff_removed=78
diff_bytes=596330
comments_count=6
commits_count=36
ci_status=success
ci_summary=total=16 success=16 failure=0 pending=0 other=0
prompt=.claude/commands/review-mr.md
blocking=false
posted_by=gh
no_comment=false
live_posting=posted

---

samorev-assisted review (AI analysis by Tanya301/samorev)