Skip to content

perf: send a message with one INSERT ... ON CONFLICT DO UPDATE - #57

Merged
lesnik512 merged 1 commit into
mainfrom
perf/message-upsert-single-statement
Oct 17, 2026
Merged

lesnik512 merged 1 commit into
mainfrom
perf/message-upsert-single-statement

Conversation

@lesnik512

Copy link
Copy Markdown
Member

CreateMessageUseCase used to make up to three trips to the database for a send: a fetch_by_idempotency_key pre-check, the insert, and after a DuplicateKeyError, a rollback and a second fetch outside the Transaction block, so outside the retried unit. A new message now costs one statement:

INSERT INTO messages (...) VALUES (...)
ON CONFLICT ON CONSTRAINT uk_messages_chat_id_idempotency_key
DO UPDATE SET idempotency_key = EXCLUDED.idempotency_key
RETURNING messages.*, xmax = 0

The no-op update makes Postgres return the existing row on a replay. A request racing an uncommitted insert with the same key waits on that row's lock and then gets the winner's row. xmax = 0 is true only for a row version this statement inserted, which gives the endpoint its 201 or 200 and limits the chats.last_message_id update to new messages. The membership check, the upsert and that update stay inside one Transaction. This follows DecksRepository.get_or_create from modern-python/fastapi-sqlalchemy-template#90.

Checked against Postgres 17.11 by hand

Case xmax = 0
fresh insert true
replay of a committed key false, original row and text
fresh insert and replay inside savepoints (the test fixture's mode) true, then false
racing an uncommitted insert that then commits false, winner's row
racing an uncommitted insert that then rolls back true, own row
replay while another transaction holds FOR KEY SHARE on the row false

Postgres does not document xmax in RETURNING, so the tests below are what would notice a change.

Tests

  • test_send_writes_the_message_with_a_single_statement: this was the red test. On main a send issues three statements that touch messages: the pre-check, the insert, and advanced-alchemy's refresh SELECT.
  • test_send_returns_the_message_a_concurrent_request_created_with_the_same_key: a second real connection holds an uncommitted insert, the use case blocks on its lock (the test polls pg_stat_activity for wait_event_type = 'Lock'), and after the commit the use case returns the first row with created=False and leaves last_message_id as NULL. It commits real rows and deletes them in finally, like tests/test_create_deck.py in the template. It also passes on main, because the old recovery path handled this case, so it guards against regressions rather than proving the change. It passed 8 runs in a row locally.
  • Removed _RacingMessagesRepository, _NeverFoundMessagesRepository and their two tests. The code they covered is gone, and fetch_by_idempotency_key had no other caller.

Trade-off worth a look

Every replay now writes a new row version and consumes a sequence value, so each retried send leaves a gap in messages.id. ADR 0001 relies on ids for client gap detection, so a client sees a gap and makes one catch-up fetch that comes back empty. Sequences already leave gaps on rollbacks; this adds one per idempotent retry. The replay row write also fires any UPDATE trigger, and messages has none.

Docs

  • New ADR 0012 records the decision and the alternatives it rejected: read-then-insert with recovery, ON CONFLICT DO NOTHING (the loser gets no row back), and advanced-alchemy's upsert (which reads first).
  • ADR 0005 now covers CreateChatUseCase only and points to 0012. ADRs 0006 and 0009 and the test-pattern note in AGENTS.md drop their references to the removed message recovery path.

CreateChatUseCase still uses the pre-check plus recovery for direct chats. Moving it over is harder because it also inserts members, so it is left for a separate change.

Verification

ruff format, ruff check, ty check, the full suite (141 passed, 100% coverage), tests/migrations, and just adr-check, all run locally against Postgres 17.

The pre-check SELECT, the insert and the DuplicateKeyError recovery re-read
become one statement. RETURNING xmax = 0 tells a new row from a replay, so
last_message_id moves only when the message was created.
@lesnik512
lesnik512 merged commit 88d6b34 into main Oct 17, 2026
4 checks passed
@lesnik512
lesnik512 deleted the perf/message-upsert-single-statement branch October 17, 2026 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant