Wait for in-flight writers before adding tables to the publication - #4787
Merged
Conversation
…dition Logical decoding evaluates publication membership per change, against the catalog state at that point in the WAL. A transaction that wrote to a table before `ALTER PUBLICATION ... ADD TABLE` committed never has those writes emitted by the replication stream, even if it commits after. They can only reach a shape via the initial snapshot, which only includes them if the writer has committed by the time the snapshot is taken. `ALTER PUBLICATION ... ADD TABLE` takes only a SHARE UPDATE EXCLUSIVE lock, which doesn't conflict with writers, so the addition could commit and the waiting shape start snapshotting while such a writer was still open. Its writes then ended up in neither the snapshot (excluded via xip_list) nor the log, and the shape was silently missing them for its whole lifetime. The first-time-add path was protected by accident: its `ALTER TABLE ... REPLICA IDENTITY FULL` takes ACCESS EXCLUSIVE in the same transaction, which can't be granted until every in-flight writer has finished. The add-only path, taken whenever the table is already REPLICA IDENTITY FULL - i.e. every re-add after the last shape on a table was removed, such as after a TRUNCATE invalidated it - had no such barrier. Take `LOCK TABLE ... IN SHARE MODE` in the same transaction as the addition, only when this transaction is the one actually adding the table. SHARE conflicts with the ROW EXCLUSIVE lock every writer holds until its transaction ends, so the addition doesn't commit until all in-flight writers have finished, while writers arriving meanwhile queue behind it and write after the addition, which is decoded normally. The lock is skipped when the table was already present: there is no gap to close, and since the savepoint rollback releases the SHARE UPDATE EXCLUSIVE lock, two concurrent configurators could otherwise both hold SHARE and deadlock upgrading to ACCESS EXCLUSIVE for the replica identity change. A lock wait timeout rolls the whole transaction back and is reported as a `DbConfigurationError` of type `table_lock_timeout`, a known retryable error, instead of leaking the raw `query_canceled` as an unexpected error. `configure_table_for_replication/4` rolls back explicitly on a nested failure so that reason propagates instead of a bare "Transaction unexpectedly rolled back". Fixes #4773 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzRyeLXJcRzHi5hxMQsgaw
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4787 +/- ##
===========================================
- Coverage 73.71% 60.05% -13.67%
===========================================
Files 88 397 +309
Lines 10094 43772 +33678
Branches 3104 12592 +9488
===========================================
+ Hits 7441 26286 +18845
- Misses 2595 17405 +14810
- Partials 58 81 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
This PR has been released! 🚀 The following packages include changes from this PR:
Thanks for contributing to Electric! |
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.
Fixes #4773
Problem
Logical decoding evaluates publication membership per change, against the catalog state at that point in the WAL. A transaction that wrote to a table before
ALTER PUBLICATION … ADD TABLEcommitted therefore never has those writes emitted by the replication stream, even if it commits afterwards. The only way for such writes to reach a shape is the initial snapshot, and the snapshot only includes them if the transaction has committed by the time the snapshot is taken.ALTER PUBLICATION … ADD TABLEtakes only a SHARE UPDATE EXCLUSIVE lock, which doesn't conflict with writers, so the addition can commit — and the shape waiting on it can start snapshotting — while such a writer is still open. Its writes then end up in neither the snapshot (excluded viaxip_list) nor the log, and the shape is silently missing them for its whole lifetime.The first-time-add path (
configure_table_for_replication/4) has always been protected by accident: itsALTER TABLE … REPLICA IDENTITY FULLtakes ACCESS EXCLUSIVE in the same transaction, which can't be granted until every in-flight writer has finished. The unprotected path is the add-only one, taken when the table is alreadyREPLICA IDENTITY FULL— i.e. every re-add after the last shape on a table was removed (TRUNCATEinvalidation as in the report, shape expiry, schema-change invalidation, …) and first adds of tables the user pre-set to FULL.Fix
add_table_to_publication/4now takesLOCK TABLE … IN SHARE MODEin the same transaction as theADD TABLE, but only when this transaction is the one actually adding the table. SHARE conflicts with the ROW EXCLUSIVE lock every writer holds until its transaction ends, so the addition doesn't commit until all in-flight writers have finished; writers that arrive meanwhile queue up behind it and write after the addition, which is decoded normally. This makes the add-only path behave the way the first-time-add path already does, with a weaker lock.ADD TABLEreports the table as already present: there is no gap to close, and — since the savepoint rollback releases the SHARE UPDATE EXCLUSIVE lock — two concurrent configurators could otherwise both hold SHARE and deadlock upgrading to ACCESS EXCLUSIVE for the replica identity change (caught by the existing concurrency test).DbConfigurationErrorof typetable_lock_timeout(a known, retryable error → 503 withRetry-Afterfor clients, no shape removal), instead of leaking the rawquery_canceledas an "unexpected error" 500. A server-sidelock_timeout(lock_not_available) is mapped the same way.configure_table_for_replication/4rolls back explicitly when a nested step fails, so the reason propagates instead of the bare{:error, :rollback}→ "Transaction unexpectedly rolled back" that a failed nested transaction would otherwise produce.Trade-off
Shape creation on a table now waits for in-flight writers on that table (bounded by the action timeout, 5 s by default). While the SHARE request is pending, new writers on the table queue behind it — the same behaviour the first-time-add path has always had. A pending SHARE request also queues behind
VACUUM/CREATE INDEX CONCURRENTLYon the table; a non-blocking variant of the barrier that avoids that is tracked separately.Independently of Electric, the same "write while unpublished, commit after" transaction triggers a walsender relation-cache bug in PostgreSQL that drops later changes to the table (fixed upstream in PG 18, back-patched in the August 2025 minor releases); the deployment guidance should recommend a PG minor that includes it.