POC: background BM25 compaction via pg_durable - #471
Draft
tjgreen42 wants to merge 60 commits into
Draft
Conversation
Add pg_textsearch.compaction_request_function, naming a function of one regclass argument that is invoked once per index that requested compaction during the transaction. In background compaction mode the spill path enqueues a request instead of compacting inline, and the pre-commit hook dispatches it, so the request commits atomically with the writer's transaction and rolls back with it. The dispatch runs after tp_bulk_load_spill_check(), because that check can itself spill and therefore enqueue a request. Two details of the pre-commit environment shape the implementation: - PreCommit_Portals() has already torn down the portal, so no active snapshot remains for SPI. One is pushed for the duration of the call. - The dispatch is wrapped in two nested subtransactions rather than one. At pre-commit the top-level blockState is TBLOCK_END, which BeginInternalSubTransaction() accepts but which the assertion at the tail of RollbackAndReleaseCurrentSubTransaction() omits from the states a parent may be in, so rolling a single subtransaction back from here crashes an assert-enabled build. Stock PostgreSQL trips the same assertion when a deferred constraint trigger written in PL/pgSQL catches an error, so this is an upstream oversight rather than a misuse. An outer subtransaction that performs no work of its own gives the inner rollback a TBLOCK_SUBINPROGRESS parent, which the assertion accepts. A failing request function is therefore isolated: it warns and the writer still commits. Parallel workers, autovacuum, recovery and prepared transactions never dispatch.
Adds scripts/durable_compaction/, the operator-side glue that lets pg_textsearch.compaction_mode = 'background' hand segment compaction to a pg_durable durable task. - 01_setup_role.sql creates the textsearch_compactor role (LOGIN, NOSUPERUSER, INHERIT), grants df usage, and gives it the index owner's privileges. - 02_wrapper.sql defines bm25_request_compaction(regclass), a SECURITY DEFINER wrapper that builds the DSL itself and drives bm25_compact_step() through df.loop(), so each merge batch runs in its own transaction. - 03_backstop.sql starts the hourly bm25_compact_pending() sweep as the compactor. - README.md documents setup order, the passwordless pg_hba requirement, shared_preload_libraries, the asynchronous bgworker init race, and the GUCs. The role cannot be called pg_textsearch_compactor: PostgreSQL reserves the pg_ role namespace.
Two gaps found while verifying the pg_durable glue end to end. pg_textsearch.segments_per_level is read in two different sessions: the writer's, when deciding whether to enqueue a request at PRE_COMMIT, and the compactor's worker session, when evaluating the df.loop condition. Setting it only on the writer produces a task that starts, finds nothing to do against the default threshold and reports 'completed' while the level counts never move -- a silent no-op that looks like success. Document the split and show setting it at a scope the worker inherits. df.nodes.error is always empty in pg_durable 0.2.6; a failed node carries status='failed' with no message, and status_details holds only the execution id. The real error is only in the Postgres server log, so the Operating query no longer selects a column that is always blank, and the limitation records where to look instead.
Add test/scripts/durable_compaction.sh, an end-to-end shell test that brings up its own PostgreSQL instance with pg_durable and pg_textsearch preloaded, wires up the three glue scripts in scripts/durable_compaction/ as an operator would, and exercises every criterion from the background compaction plan: atomicity of the enqueue-at-commit, that background mode actually drops segment counts, inline vs background write latency, the failure/self-heal path when compactor ownership is revoked and restored, a multi-level cascade that splits across transactions while a concurrent writer commits, a writer with EXECUTE-only privileges still enqueuing an attributed task, and the scheduled backstop sweep compacting an index with compaction_mode = 'off' and no writer involvement. Add a test-durable Makefile target that runs the new script. It is deliberately not wired into test-all/test-shell since CI has no pg_durable. Parameterize scripts/durable_compaction/03_backstop.sql with an optional -v cron=... override (default unchanged) so the test can run the sweep on a fast cadence; the value is threaded through a custom GUC because psql's :'var' interpolation does not reach inside the script's DO $$ ... $$ body.
The durable compaction test invoked initdb/pg_ctl/psql/createdb bare, so it silently used whichever Postgres happened to be first on PATH. On a machine whose PATH leads with a system Postgres the cluster died at startup with only "could not access file: pg_durable" in a log under a temp directory, several layers away from the real cause. Pin every binary to pg_config --bindir, matching the convention already used in shutdown_spill.sh. That is the correct coupling: the install pg_config points at is the one make install populated, and therefore the only $libdir where pg_textsearch.so is discoverable. Add a preflight check that both modules are present in pg_config --pkglibdir and, if not, name the missing module, the directory searched and the pg_config responsible. Also rename the backstop's cadence GUC from pg_durable_test.cron to pg_textsearch_backstop.cron; the override is a supported feature of a shipped script, not a test-only hook, and the old name implied otherwise.
The latency benchmark only averaged spill-round transactions, and most spill rounds merge nothing. That measures the fixed enqueue cost background mode adds to every request and almost nothing else, so it concluded background was uniformly slower and that no crossover scale existed. The enqueue cost is real -- about 0.06s -- but it is not the cost this design exists to remove. Add a second scenario that isolates the transaction that does cross the threshold: pile up segments with compaction off, then time the single commit that triggers the cascade. Over 200,000 rows in eight segments it measures 0.982s inline against 0.084s background, and at segments_per_level = 2, forcing a deeper cascade, 2.794s against 0.097s. Both modes converge on the same final level layout, so the background path defers the merge rather than skipping it. Two bugs made the first version of this scenario report nonsense. memtable_pages_threshold and bulk_load_threshold are PGC_SUSET, so the per-session SET as app_owner failed 18 times; because the failures happened inside a command substitution the script still exited 0 and printed 0.006s for both modes. The thresholds now go through ALTER SYSTEM like every other GUC here, and the scenario asserts it actually built the segments it is about to merge instead of silently timing an empty one. Correct the report and the operator doc accordingly: state the trade as a latency-variance trade with a table of both cases, and tell operators the scale below which background mode costs more than it saves.
Add a self-contained, three-act demo of pg_durable-driven background BM25 segment compaction: - Act 1: an inline-mode INSERT blows through a calibrated statement_timeout; the merge still completes (segment merges are non-transactional GenericXLog page mutations), so the client pays the full cost and still gets an error. - Act 2: the identical insert under background mode commits well inside the same timeout, and the merge is verified to actually happen via level counts (not just instance status). - Act 3: concurrent ranking-invariance, no-torn-reads, atomicity, and writer-latency checks against a live cascade, all as hard assertions confirmed to run while the cascade is still 'running'. Also documents two findings from running it: inline mode's statement_timeout does not roll back already-applied merge work, and the measured max concurrent-writer latency (2.802s) under a held LW_EXCLUSIVE merge batch, in docs/background_compaction_report.md.
The demo established that cancelling an inline compaction leaves the merge applied, and recorded the root cause as undetermined. It is determinable, and it is sharper than "segment pages are not transactional". merge.c calls CHECK_FOR_INTERRUPTS() every 1000 terms, but it does so while the backend holds the per-index LW_EXCLUSIVE, and LWLockAcquire() calls HOLD_INTERRUPTS() precisely so that interrupts cannot interfere with shared memory manipulation (lwlock.c:1216-1219). CHECK_FOR_INTERRUPTS() is a no-op while InterruptHoldoffCount is non-zero, so every one of those checks is dead code until LWLockRelease() resumes interrupts. Corpus size is not the limiting factor: the demo dictionary is about 25,600 terms, so the checks are reached thousands of times and simply cannot act. This matters beyond the demo. statement_timeout cannot bound a client's exposure to an inline merge -- it waits for the whole merge, receives an error, and the compaction is applied anyway, so the write is lost and the work is not. A writer stalled behind a merge batch is uncancellable for the same reason. That is a stronger argument for background mode than the latency table. Record the consequences, including that the interrupt checks are misleading as written and become meaningful only once a merge builds its output without holding the lock, which is the direction "Future work: non-blocking merges" already describes. Also report the concurrent-writer stall as a range. An independent rerun measured 4.471s where the first measured 2.802s; quoting one figure implied a stability the measurement does not have.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
pgspot rejected the extension SQL: bm25_needs_compaction(), bm25_indexes_needing_compaction() and bm25_compact_pending() are the first SQL- and plpgsql-bodied functions in this extension, so they are the first to be subject to its search_path rules. Without an explicit SET clause every unqualified operator and reference in their bodies is resolvable through the caller's search_path, which is a search_path injection vector; CAST(c.oid AS regclass) was reported as a hard error and eleven further findings as warnings, and pgspot fails on either. Pin search_path on all three, and additionally qualify the two references pgspot named (pg_catalog.unnest, pg_catalog.regclass) so the error stays fixed even if a SET clause is later dropped. The remaining findings were operator references, which the pin resolves without having to spell every operator as OPERATOR(pg_catalog.=). Apply the same change to the 1.4.0 upgrade script, which ships these definitions verbatim, and extend both pgspot workflows to check the upgrade script into the current version -- previously only the base install file was checked, so an upgrading user could have received SQL that CI never looked at. Already-released upgrade scripts stay excluded: they are frozen and carry pre-existing warnings. The compaction test replaced bm25_indexes_needing_compaction() with a stub that referenced its target index unqualified. A stub without a SET clause inherits the pinned path of its plpgsql caller, so the name no longer resolved and the FOR loop query failed during planning -- outside the per-index exception handler the test exists to exercise. Give the stub the same pinned path as the function it stands in for and qualify the reference, restoring the intended warn-and-continue path. Note that a SET clause makes a SQL function ineligible for inlining. Both functions are called once per sweep or loop iteration, so this costs nothing measurable here.
This was referenced Aug 29, 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
1 participant
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.
Status
This branch is a preserved proof of concept and umbrella, not a merge
candidate. It demonstrates background BM25 compaction through pg_durable and
records the design, measurements, and hardening work from the experiment.
The mergeable implementation is being rebuilt as five focused changes:
The extraction contract is
docs/superpowers/specs/2026-08-29-background-compaction-pr-series-design.md.POC behavior
Spills in background mode record transaction-local requests. At PRE_COMMIT,
the configured callback submits pg_durable work with
transaction_mode => 'new'. The durable stepped cascade performs one mergebatch per transaction.
Default behavior is unchanged:
pg_textsearch.compaction_modedefaults toinline.Documentation
docs/background_compaction.mdscripts/durable_compaction/README.mdImportant limitations
failure-resilient recurrence from Add a node failure policy: retries, and what happens when retrying doesn't help microsoft/pg_durable#354.
CREATE EXTENSION.Extraction PRs
Upstream dependency:
microsoft/pg_durable#354.