fix: refresh partition materialized views concurrently to avoid ACCESS EXCLUSIVE locks#460
Open
fabito wants to merge 1 commit into
Open
fix: refresh partition materialized views concurrently to avoid ACCESS EXCLUSIVE locks#460fabito wants to merge 1 commit into
fabito wants to merge 1 commit into
Conversation
…S EXCLUSIVE locks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Related Issue(s)
Fixes #311
Description
update_partition_stats()andcreate_partition()refresh thepartitionsandpartition_stepsmaterialized views with a blockingREFRESH MATERIALIZED VIEW, which takes anACCESS EXCLUSIVElock. Sincesearch()readspartition_stepson every call, this causes two failure modes in production:ACCESS EXCLUSIVElock is WAL-logged (rmgr: Standby, desc: LOCK) and replayed on replicas, where it cancels any in-flight search still holding a relation lock aftermax_standby_streaming_delaywithcanceling statement due to conflict with recovery — User was holding a relation lock for too long.We hit mode 2 in production (pgstac 0.9.11 on Aurora PostgreSQL 17.7, reader endpoint serving stac-fastapi-pgstac): with
pgstac.use_queue=trueand a scheduledrun_queued_queries()drainer, every busy drain emittedACCESS EXCLUSIVElocks on the matviews and deterministically cancelled long-running multi-farm searches on the reader. Log correlation showed every cancellation burst landing inside a busy drainer run, exactlymax_standby_streaming_delayafter the standby lock record arrived.Change
partition_steps (name)(partitionsalready has one on(partition)), whichREFRESH ... CONCURRENTLYrequires.nameis the partition's table name, so it is inherently unique.refresh_partition_matviews(), which refreshes both views withREFRESH MATERIALIZED VIEW CONCURRENTLY— this takes only anEXCLUSIVElock, which does not conflict with readers'ACCESS SHARElocks, eliminating both the deadlock and the standby-cancellation paths. It falls back to a blocking refresh (previous behavior, with aNOTICE) for the casesCONCURRENTLYcannot handle, e.g. a never-populated materialized view.update_partition_stats(),create_partition()).Answering the open questions from the issue thread:
REFRESH ... CONCURRENTLYrun inside plpgsql / the queue procedure? Yes — verified on PostgreSQL 17: unlikeCREATE INDEX CONCURRENTLY, concurrent matview refresh is a single-transaction operation and works both inside plpgsql functions and viaEXECUTEin a procedure that uses transaction control (run_queued_queries()). TheCONCURRENTLY-stripping in the queue runner is unaffected: it operates on queued query text (needed forCREATE INDEX CONCURRENTLY, which genuinely cannot run there), not on statements inside function bodies.AS SELECTwithoutWITH NO DATA); the fallback covers restore-style edge cases where a view exists unpopulated.Trade-off: a concurrent refresh does more work than a blocking one (temp-table diff against the unique index). Both views have one row per partition, so the extra cost is negligible relative to the
ANALYZEalready performed byupdate_partition_stats().Notes on generated files and the incremental migration
pgstac--unreleased.sqlandpgstac.sqlwere regenerated withscripts/stageversion. Thepgstac--0.9.11--unreleased.sqlincremental is deliberately not regenerated here, and two pre-existing issues surfaced while testing it (both reproduce on cleanmain, independent of this change):scripts/test --migrationsonmainfails post-upgrade withfunction pgstac.jsonb_common_paths_agg(jsonb) does not exist.scripts/stageversionproduces an upgrade script that fails withcannot drop table search_wheres because other objects depend on it— the pgpkg-generated drop doesn't account for dependent objects, so the regenerated incremental appears to need hand-editing.Since both predate this PR, I've left the incremental as-is rather than bundle a large, partly-broken regeneration into an unrelated fix — happy to help with it separately.
scripts/test --pgtappasses: 360 assertions, including new checks for thepartition_steps (name)unique index and therefresh_partition_matviews()function.Type of change
Checklist
partition_steps, helper function exists)