Repository navigation
perf: send the publish NOTIFY in the INSERT's RETURNING - #235
Merged
Merged
Conversation
Benchmark gate✅ gate passed
Gated (fails the build): |
This was referenced Oct 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For 0.15.1, no issue: a single
publishnow sends its NOTIFY in the INSERT'sRETURNING, saving one round trip per publishing transaction.Why
A transaction's first
publishto a queue ran the INSERT and then a separateSELECT pg_notify(...). That second statement is one more round trip to Postgres for every publishing transaction, and it's the common case: one publish per business transaction.The change
(timer_id set, activate_in set). The notify variants returnpg_notify('outbox_<table>', outbox.queue)next to the id. The channel is a bound constant and the payload is the inserted row's ownqueuecolumn, so no new parameter is sent per publish.ON CONFLICT DO NOTHINGskips a duplicatetimer_id, no row comes back, so no NOTIFY fires, the same as before.publish_batchkeeps its separateSELECT pg_notify: it runs an executemany, which has no singleRETURNINGto carry the NOTIFY. It shares the same memo, in both directions.Measured
Median time of the
publishcall inside an open transaction, against real Postgres in docker, 3000 transactions per case, three runs each:timer_idThat's about a third off each publishing transaction on a local connection. The saving is one network round trip, so it grows with the latency to Postgres. The benchmark gate passes: the producer's
insert_callsand tuple counters are unchanged, and its separateselect_callsgo away.Tests
test_publish_notifies_in_the_insert_round_tripchecks that two publishes in one transaction run exactly two statements, withpg_notifyonly in the first. It failed before the change, which ran three.test_notify_sent_after_a_timer_conflict_in_the_same_transactionuses a realLISTENconnection. It covers the memo edge case this change could have introduced: a skipped duplicate timer followed by a normal publish still delivers one notification.test_notify_dedup_spans_publish_and_publish_batchcovers a batch after a single publish in one transaction.SELECT pg_notifynow assert the single statement.Docs
how-it-works.mdandperformance.mddescribed the separate statement. They now say a single publish carries the NOTIFY inRETURNINGandpublish_batchsends one extra statement.Also in this PR: coverage uses
sys.monitoringThe first CI run failed on two things.
pg_notifybind without a::VARCHARcast, and one new unit-test assertion expected the cast. The assertion now accepts both forms.pytestjob fell under 100% becausetest_batched_flush_dlq_row_stays_inlinesometimes reported two lines as never run. Those are the loop that publishes the successful rows.The test proves those lines run: with an extra assertion that all four rows arrived, it passed every time while the lines were still reported missed. It's coverage.py's default C tracer losing lines in this async test, and it isn't new. Locally,
mainshowed it in 3 of 15 runs of that test. With this branch's timing it showed in 7–8 of 15, and with coverage'ssys.monitoringbackend in 0 of 15.So
[tool.coverage.run]now setscore = "sysmon":sys.monitoring, andjust test-branchasks for branch coverage, whichsysmoncan't measure before 3.14. In both cases coverage falls back to the C tracer with ano-sysmonwarning, which is disabled.Verification
ruffandtyare clean, the docs build, andjust bench-checkpasses.