perf(postgres): batch bulk-submit ingest and pipeline per-resource writes - #880
Merged
Conversation
…ites The Postgres ingest paths ran at ~7-10 resources/s — every Synthea transaction Bundle over ~330 entries timed out against the default 30-second request timeout, and the 15.5k-line #448 bulk cohort took 26-37 minutes (#872). Two costs, two fixes: Autocommit overhead: process_entries now runs one transaction per batch, mirroring the SQLite batching from #815 — entry rows, history, search-index writes, rollback records, and per-line receipts commit together, so the rollback log can never diverge from what was written. Each entry runs under a savepoint: on Postgres a failed statement aborts the whole transaction, and the savepoint contains a failure to its one entry the way SQLite's per-statement independence does. Round trips: batching alone barely moved the needle (~10/s) because the cost was serial round trips, not fsyncs — a resource with 15+ indexed parameters paid 15+ sequential awaits for its search-index rows alone. Those inserts, the create's resource+history pair, and the change+receipt bookkeeping pair are now pipelined on the transaction's connection via joined futures; tokio-postgres sends them back-to-back and statements still execute in order, inside the transaction. Measured on the #448 dataset (postgres:16 in local Docker, release): bulk -submit, 15,514 lines: 1587s -> 581s (~27/s, 2.7x) transaction Bundle, 927 entries: 80.6s -> 28.8s transaction Bundle, 2836 entries: 288.7s -> 75.7s Typical Synthea bundles (300-900 entries) now land inside the default timeout; only multi-thousand-entry outliers still need it raised. New integration test pins the batch contract on a real Postgres: bookkeeping commits with the entries, and a soft-deleted-id failure is contained to its savepoint while the rest of the batch lands. Closes #872
9 tasks
Main rewrote all three conflicting regions of the Postgres transaction — creates are now buffered and flushed in batches, index writes go through the denormalized composite layout, and the write path uses cached statements — so each conflict takes main's side; the PR's per-statement pipelining there is superseded. The batching changes what the bulk-submit savepoint loop can assume: a create's conflict is now discovered at flush, which poisons the transaction, and a threshold flush inside entry N's savepoint would put earlier entries' rows under it. So the savepoint handling moves onto the transaction: `savepoint` flushes what came before, `release_savepoint` flushes the entry's own creates so its conflict is raised inside its savepoint, and `rollback_to_savepoint` drops the buffer and lifts the conflict, since the rollback undoes the only batch that could have held it. Claude-Session: https://claude.ai/code/session_011L8HSXPJ745DWBwJc3SzPc
smunini
approved these changes
Sep 2, 2026
angela-helios
pushed a commit
that referenced
this pull request
Sep 3, 2026
… manifest `postgres_bulk_submit_import_directives_round_trip` called `claim_next_manifest` and asserted the claimed view carried the directives it had just set. The claim queue is cross-tenant and ordered by `added_at`, the test binary shares one container database, and the batch test that #880 added leaves its manifest as `processing` with no lease — which the claim query treats as an orphan to reclaim. Whenever that manifest was added first, the directives test claimed it instead of its own and failed with `left: []`; main's coverage job has been red on most runs since. The test now claims through `claim_specific_manifest`, the submit-side twin of the export tests' `claim_specific`: loop until the target manifest comes back, hold any foreign lease picked up along the way so it cannot be re-claimed, then release those back to the queue. The batch test's comment no longer presents its `process_entries` call as a defence against concurrent claims. No product code changes. Tests: the two submit tests pass five consecutive runs together; the full postgres_tests binary passes (156).
angela-helios
pushed a commit
that referenced
this pull request
Sep 3, 2026
… manifest `postgres_bulk_submit_import_directives_round_trip` called `claim_next_manifest` and asserted the claimed view carried the directives it had just set. The claim queue is cross-tenant and ordered by `added_at`, the test binary shares one container database, and the batch test that #880 added leaves its manifest as `processing` with no lease — which the claim query treats as an orphan to reclaim. Whenever that manifest was added first, the directives test claimed it instead of its own and failed with `left: []`; main's coverage job has been red on most runs since. The test now claims through `claim_specific_manifest`, the submit-side twin of the export tests' `claim_specific`: loop until the target manifest comes back, hold any foreign lease picked up along the way so it cannot be re-claimed, then release those back to the queue. The batch test's comment no longer presents its `process_entries` call as a defence against concurrent claims. No product code changes. Tests: the two submit tests pass five consecutive runs together; the full postgres_tests binary passes (156).
angela-helios
pushed a commit
that referenced
this pull request
Sep 3, 2026
… manifest `postgres_bulk_submit_import_directives_round_trip` called `claim_next_manifest` and asserted the claimed view carried the directives it had just set. The claim queue is cross-tenant and ordered by `added_at`, the test binary shares one container database, and the batch test that #880 added leaves its manifest as `processing` with no lease — which the claim query treats as an orphan to reclaim. Whenever that manifest was added first, the directives test claimed it instead of its own and failed with `left: []`; main's coverage job has been red on most runs since. The test now claims through `claim_specific_manifest`, the submit-side twin of the export tests' `claim_specific`: loop until the target manifest comes back, hold any foreign lease picked up along the way so it cannot be re-claimed, then release those back to the queue. The batch test's comment no longer presents its `process_entries` call as a defence against concurrent claims. No product code changes. Tests: the two submit tests pass five consecutive runs together; the full postgres_tests binary passes (156).
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
2 participants
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.
Closes #872
What
The Postgres ingest paths ran at ~7–10 resources/s: every Synthea transaction Bundle over ~330 entries 408'd against the default 30s request timeout, and the #448 bulk cohort (15,514 lines) took 26–37 minutes. Two distinct costs, two fixes:
1. Autocommit overhead → one transaction per batch (the #815 port).
process_entrieswraps each batch in aPostgresTransaction: entry rows, history, in-transaction search-index writes, rollback records, and per-line receipts commit together — the rollback log can never diverge from what was actually written. Postgres wrinkle the SQLite version didn't have: a failed statement aborts the whole transaction, so each entry runs under aSAVEPOINT— a bad entry rolls back to its savepoint, is recorded as that entry's error, and the rest of the batch lands.2. Serial round trips → pipelining. Batching alone measured ~10/s: the real cost was round trips, not fsyncs — a resource with 15+ indexed search parameters paid 15+ sequential awaits for its
search_indexinserts alone. Those inserts (try_join_all), the create's resource+history pair, and the change+receipt bookkeeping pair are now polled concurrently on the transaction's connection; tokio-postgres pipelines them back-to-back, and statements still execute in order, inside the transaction. This also speeds the FHIR transaction-Bundle path, which sharesPostgresTransaction.Measured (#448 dataset, postgres:16 in local Docker, release build)
$bulk-submit, 15,514 linesTypical Synthea bundles (300–900 entries) now land inside the default 30s timeout out of the box; multi-thousand-entry outliers still need
HFS_REQUEST_TIMEOUTraised. Further headroom (multi-row VALUES for the index, prepared-statement reuse, COPY) noted as follow-up potential — the remaining per-entry round trips are the read/exists checks and the savepoint pair.Tests
New
postgres_bulk_submit_batch_commits_bookkeeping_and_contains_errorson a real testcontainers Postgres: a 3-entry batch with a create, an update, and a soft-deleted-id failure — the failure is contained to its savepoint, the other two commit with their receipts and rollback records, and the counts reconcile. Fullpostgres_testssuite: 122/122 (also fixed a pre-existing shared-database race my first version exposed: a pending manifest left claimable could be grabbed by the import-directives test'sclaim_next_manifest).