Conversation
fafc9e1 to
9a28fc2
Compare
|
Force-pushed with two fixes from second review round: 1. Trimmed both sections.
2. Corrected the 'e2e is floored at ~1 s' implicit claim. Previous wording risked reading as "PgQue cannot do sub-ms e2e." It can — the 1-second default is a
Plus the metadata-table bloat caveat with pointer to #61 and PR #62. Framing attribution — added Hannu Krosing (ex-Skype, original PgQ context) alongside the HN thread in the Provenance paragraph. One thing I did NOT touch that you may want to revisit: the existing "Latency trade-off" section above "Three latencies" still says "If your top priority is single-digit-millisecond dispatch, PgQue is the wrong tool." That's now inconsistent with the "tunable, not floored" framing in the new section. Happy to soften it in a follow-up if you want — kept this PR scoped to the new section only. |
20ece4d to
e43dd57
Compare
|
Two cleanups:
|
e43dd57 to
18a0263
Compare
|
Three docs cleanups:
|
18a0263 to
defdc4d
Compare
|
Terminology fix: "poll-on-demand" → "UPDATE/DELETE" (the canonical framing — those systems' distinguishing trait is UPDATE/DELETE-based consumption, which creates dead tuples). |
Name the three distinct latencies in any Postgres queue and explain why PgQue's batch-ticker model makes #1 and #2 sub-ms while bounding #3 by the tick cadence (not by load). Addresses recurring confusion about the apparent contradiction between sub-ms consumer-path latency and the ~1 s end-to-end delivery bound. - README.md: brief paragraph + 3-bullet list, new "Three latencies" subsection between "Latency trade-off" and "Comparison". - docs/pgq-concepts.md: detailed version with per-latency physics, tick-frequency trade-off table, comparison to pgmq's poll-on-demand model, when-to-pick guidance, provenance link. Uses actual pgque.sql column names: queue_ticker_max_lag (3s), queue_ticker_idle_period (1min idle-decelerator), queue_ticker_max_count (500). The 1-second cadence comes from the pg_cron schedule set by pgque.start(), not from queue_ticker_idle_period.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
defdc4d to
bf560ae
Compare
REV review — PR #68
|
| Blockers | 0 |
| Major | 0 |
| Minor | 2 |
| Nits | 2 |
5-perspective summary
Security: N/A. No suspicious links. Provenance link is news.ycombinator.com/threads?id=samokhvalov — author's own HN handle, fine.
Bug hunter: Function names and config knobs verified against the v0.1 install:
pgque.send,pgque.insert_event,pgque.next_batch,pgque.get_batch_events,pgque.set_queue_config()— all matchdocs/reference.md.queue_ticker_max_lag(default3 seconds),queue_ticker_max_count(default 500),queue_ticker_idle_period(default1 minute) — all matchsql/pgque.sql:110-112exactly.- Bench numbers
~86k ev/sand~2.4M ev/smatchdocs/benchmarks.md:45,48(85,836 ev/sand~2.4M ev/s). - Math sanity:
1 s / 100 = 10 ms,1 s / 1000 = 1 ms— staggered table consistent. - No SQL/Python code blocks in the new doc, so no syntax to break.
Test analyzer: N/A (docs-only).
Guidelines (CLAUDE.md):
- Lowercase SQL, snake_case, schema-qualified
pgque.X— compliant throughout. - Conventional Commits:
docs: promote three-latencies to dedicated page— compliant. - Binary units: only
MiB/sappears (transitively, via reference to bench doc); no inline storage sizes in new prose to violate the rule. - README structure: 5-line tease (1 intro + 3-bullet list + link) does not duplicate the dedicated page — passes the tease-not-duplicate test.
docs/README.mdindex updated.docs/pgq-concepts.mdno longer duplicates the prose; replaced with a clean cross-reference. Good.
Docs:
- Flow is clear: problem framing → 4-column table naming all three latencies → why Add pgque-specific schema additions (config table, roles, queue_max_retries) #3 dominates → tunable-not-floored subsection → load behavior contrast → when-to-pick → provenance.
- Each of the three latencies named, defined, with example numbers.
- Tone matches the post-docs: sentence-level polish across README + docs/ #65 sentence-level polish.
- Generalized comparison ("UPDATE/DELETE-based designs") avoids naming a single competitor — good neutral framing.
Anti-leak: grep -iE \"gitlab|sahmed|artifact[_-]?registry|@AR|wi[ -]?#?7[67]|round[ -]?8|R8\" on the diff — clean.
Rebase integrity (force-with-lease check): Branch has two commits (1206929 initial → bf560ae promote). The rebased tip diff against main shows the full consolidated state: 46-line standalone doc with all 3 sections + tables + provenance, ToC entry, and the cross-reference stub in pgq-concepts.md. No content scrambling or dropped sections.
CI: All green (claude-review, client-smoke, test 14-18, verify).
Minor
-
Stale PR body. The PR description still references the older inlined-into-pgq-concepts.md structure ("~142 lines", "detailed version appended", "keep it as a section of pgq-concepts.md (current choice)"), which no longer matches the branch. The current branch is the trimmed standalone-page version. Consider refreshing the description before un-drafting so reviewers and future readers see what actually shipped.
-
Self-flagged inconsistency between sections. The author's own comment notes that the upstream
## Latency trade-offsection still says "If your top priority is single-digit-millisecond dispatch, PgQue is the wrong tool", which contradicts the new "tunable, not floored" framing. Out of scope here, but worth a follow-up PR (a one-line softening to something like "…the default config is the wrong tool — sub-ms e2e is achievable, see Three latencies" would close the loop).
Nits
-
"≈ tick period + consumer poll interval" in the row-3 table cell is correct but introduces a poll-interval term that isn't elaborated until the "Load behavior" section several paragraphs later. A reader scanning only the table may briefly wonder where the consumer poll comes from. Optional: parenthetical "(see below)" or move to plain "≈ tick period" in the table and explain the consumer-poll addition in body text.
-
Guidance table phrasing. Row 1 says
pgqd-compatible— not wrong, butpgqdis unexplained at first encounter and may be unfamiliar to readers arriving from the README tease. Consider "matches PgQ's pgqd default" or linking to where pgqd is introduced.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Addresses recurring confusion (including a HN thread today: https://news.ycombinator.com/threads?id=samokhvalov) about the apparent contradiction between PgQue's sub-ms consumer-path latency and the ~1-second end-to-end delivery bound.
Names the three distinct latencies in any event queue and explains PgQue's trade-off explicitly:
next_batchreturning an already-built batch. Sub-ms.pg_cron), not by load. This is the design property that polling queues lack.Changes
README.md: new "Three latencies" subsection between "Latency trade-off" and "Comparison" — one paragraph + 3-bullet list + link to the concept doc.docs/pgq-concepts.md: detailed version appended to the existing concept doc — per-latency physics with preliminary bench numbers, tick-frequency trade-off table (1 s / 250 ms / 100 ms / sub-100 ms), comparison to pgmq's poll-on-demand model including the MVCC/bloat failure mode, when-to-pick guidance, provenance link to the HN thread.Uses the real config knob names from
sql/pgque.sql:queue_ticker_max_lag(default3 seconds) — safety capqueue_ticker_idle_period(default1 minute) — idle deceleratorqueue_ticker_max_count(default 500) — batch-size cappg_cronschedule set inpgque.start()(default1 second)No SQL files modified. No invented features.
Test plan
docs/three-latencies.mdwith a redirect from the concept doc, or keep it as a section of pgq-concepts.md (current choice)Draft — author review before merge.