Re: Per-table resync for logical replication subscriptions

Hi Masahiko,

Thank you for the review. Attached is v4.

Taking the design question first, since it is the one that could have
changed the shape of the patch.

> Or is it worth considering an alternative design that the tablesync
> worker truncates the table in the same transaction of the COPY?

I looked into this and I do not think it works, for three reasons. I would
be glad to be told if I have missed something.

First, it would make any table that is the target of a foreign key
impossible to resync. heap_truncate_check_FKs() rejects on the existence
of the constraint, not on whether the referencing table has rows.

A tablesync worker is responsible for exactly one relation, so it can only
ever issue a single-relation truncate, which for such a table always
fails. Truncating all of the named relations in one statement is also what
makes the multi-table form you asked for in the first review work for a
set connected by foreign keys.

Second, the lock would span the whole COPY, and it would stall the
subscription.

The first half follows from TRUNCATE taking ACCESS EXCLUSIVE and from
"Once acquired, a lock is normally held until the end of the transaction"
, so the lock is not released when the TRUNCATE statement
finishes. Measured, with a 30 million row COPY after the TRUNCATE in one
transaction.

The second half I also tested. Holding
AccessExclusiveLock on one relation of a subscription (via a prepared
transaction, so no session is involved) while the publisher writes to that
relation and to a second, unlocked one:

elapsed rows in 'b' apply worker waiting on ungranted lock on 'a'
1s 0 Lock:relation RowExclusiveLock
...
6s 0 Lock:relation RowExclusiveLock
after ROLLBACK PREPARED: a=1 b=1

Table b is never locked, yet it receives nothing for as long as the lock is
held, because apply is single-threaded per subscription. The reason it
blocks even though the relation is still syncing and its changes would be
skipped is that apply_handle_insert() locks first and decides afterwards:

rel = logicalrep_rel_open(relid, RowExclusiveLock);
if (!should_apply_changes_for_rel(rel))

This is also the choice LogicalRepSyncTableStart() documents:

/*
* Use a standard write lock here. It might be better to disallow access
* to the table while it's being synchronized. But we don't want to
block
* the main apply process from working and it has to open the relation
in
* RowExclusiveLock when remapping remote relation id to local one.
*/
rel = table_open(MyLogicalRepWorker->relid, RowExclusiveLock);

Third, truncate failures would move from the user's session into a
background worker. Today a foreign key problem, a missing privilege or a
lock timeout is reported to the client and nothing has changed; in the
alternative they would surface inside a worker that fails and retries.

There is also a smaller structural cost as far as i could see: srsubstate
'i'
currently means "new relation, never copied", and it would have to also
mean
"existing relation, truncate before copying". Those cannot be conflated,
because a
user may deliberately preload a table before adding it to a subscription,
so a new state or column would be needed.

> I think it's better to drop the replication slots at the very end of
> AlterSubscription_refresh_table()

Done. The slots are dropped after the truncate and after the state reset,
as the last thing the function does, since dropping a remote slot is not
transactional and everything above it can still fail for ordinary reasons.
The relations needing a slot dropped are collected in the first pass,
which made your sixth point below fall out naturally.

> Since REFRESH TABLE always copies the table data, I think we need the
> following twophasestate check

Added. There is no copy_data = false escape here, since re-copying is the
entire purpose of the command, so the wording differs slightly from
REFRESH PUBLICATION:

ERROR: ALTER SUBSCRIPTION ... REFRESH TABLE is not allowed when
two_phase is enabled
HINT: Use ALTER SUBSCRIPTION ... SET (two_phase = false), or use
DROP/CREATE SUBSCRIPTION.

> I think we should take an AccessExclusiveLock when opening the table
> instead of escalating the lock level during the truncation.

Done, the relation is locked at that level in the resolution pass.

> we should use their OID rather than passing a list of RangeVar ... So we
> should use ExecuteTruncateGuts() instead.

Done, and two things came out of it worth flagging.

ExecuteTruncateGuts() performs no privilege check on the tables; the check
in the ExecuteTruncate() path comes from RangeVarCallbackForTruncate(), so
calling the guts directly would have silently dropped it. The command now
requires TRUNCATE on each named relation explicitly, with tests for the
denied and the granted case.

The other is partitioned tables. Since ExecuteTruncateGuts() truncates
exactly what it is handed, the partitions of a named partitioned table
have to be collected too, which the patch does the same way
apply_handle_truncate() does, skipping other backends' temp tables.

> I think we need to check if the apply worker actually stopped, in
> addition to this check. Sub->enabled being false doesn't guarantee that
> the apply worker is not working.

Agreed, and this one grew. Checking the apply worker alone is not enough,
because a tablesync worker outlives its apply worker. Measured on a 1 GB,
4 million row table, disabling the subscription mid-copy:

elapsed apply workers tablesync workers rows committed
0s 0 1 0
...
9s 0 1 0
10s 0 0 4000000

The apply worker is gone immediately, while the tablesync worker runs for
another ten seconds and then commits the whole copy. Any check that looked
only at WORKERTYPE_APPLY would have passed at 0s, with a worker still
holding the relation and about to write four million rows into it. v4 waits
for all worker types instead:

if (logicalrep_workers_find(subid, false, true))
ereport(ERROR, ...
errmsg("cannot %s when logical replication worker is still
running", ...

only_running is false, as DROP SUBSCRIPTION does for the same reason: a
worker that is in_use but has not attached yet is the dangerous one, since
it is about to read the relation state we are rewriting. I checked this
cannot lock a user out permanently, as WaitForReplicationWorkerAttach()
either observes the attach or calls logicalrep_worker_cleanup().

On your point, using one connection to the publisher for all of the
relations: done, as noted above. A single walrcv_connect() serves every
slot, and the common case where all named relations are ready needs no
publisher connection at all.

> Please update psql's tab-completion for the new syntax.

Done, including completion of table names after the keyword and after a
comma, verified by hand against a readline build.

Not something was raised, but v3 had no documentation at all. v4 adds a
REFRESH TABLE entry to alter_subscription.sgml covering the requirements,
the all-or-nothing behaviour of the list, and the partition, inheritance
and shared-relation behaviour.

Regards,
Cagri Biroglu

On Mon, Aug 10, 2026 at 11:30 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
wrote:

> On Mon, Aug 10, 2026 at 1:53 AM Cagri Biroglu <cagri(dot)biroglu(at)adyen(dot)com>
> wrote:
> >
> > Hi again,
> >
> > Thanks, and thanks for picking this up. I've registered it in the open
> > CommitFest (PG20-2).
>
> +1
>
> >
> > Attached is v3, which is v2 rebased onto current master. Master removed
> > Subscription.conninfo, so the one code change is that the mid-sync path
> > now resolves the connection string with SubscriptionConninfo(sub). It
> > does that at the point of use rather than up front, because the common
> > case here is a relation in ready state, which needs no publisher
> > connection at all. Nothing else changed, so your review of v2 still
> > applies.
> >
>
> Thank you for updating the patch! I've reviewed the v3 patch and here
> are review comments:
>
> ---
> + PG_TRY();
> + {
> + ReplicationSlotNameForTablesync(sub->oid, relid,
> syncslotname,
> + sizeof(syncslotname));
> + ReplicationSlotDropAtPubNode(wrconn, syncslotname, true);
> + }
> + PG_FINALLY();
> + {
> + walrcv_disconnect(wrconn);
> + }
> + PG_END_TRY();
>
> I think it's better to drop the replication slots at the very end of
> AlterSubscription_refresh_table() if possible because
> ExecuteTruncate() can fail for many reasons (e.g., foreign key
> constraints, insufficient privileges etc.).
>
> Or is it worth considering an alternative design that the tablesync
> worker truncates the table in the same transaction of the COPY?
>
> ---
> Since REFRESH TABLE always copies the table data, I think we need the
> following twophasestate check that is done in REFRESH PUBLICATION
> command:
>
> if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED &&
> opts.copy_data)
> ereport(ERROR,
> (errcode(ERRCODE_SYNTAX_ERROR),
> errmsg("ALTER SUBSCRIPTION ... REFRESH PUBLICATION with
> copy_data is not allowed when two_phase is enabled"),
> errhint("Use ALTER SUBSCRIPTION ... REFRESH PUBLICATION
> with copy_data = false, or use DROP/CREATE SUBSCRIPTION.")));
>
> ---
> +
> + relid = RangeVarGetRelid(rv, AccessShareLock, false);
> +
>
> I think we should take an AccessExclusiveLock when opening the table
> instead of escalating the lock level during the truncation.
>
> ---
> + tstmt = makeNode(TruncateStmt);
> + tstmt->relations = truncrels;
> + tstmt->restart_seqs = false;
> + tstmt->behavior = DROP_RESTRICT;
> + ExecuteTruncate(tstmt);
>
> Since we already resolve individual specified table names we should
> use their OID rather than passing a list of RangeVar to let
> ExecuteTransaction() resolve the OIDs again. So we should use
> ExecuteTruncateGuts() instead.
>
> ---
> + if (sub->enabled)
> + ereport(ERROR,
> +
> errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("%s is not allowed for enabled
> subscriptions",
> + "ALTER SUBSCRIPTION ... REFRESH TABLE"),
> + errhint("Disable the subscription with
> ALTER SUBSCRIPTION ... DISABLE first."));
>
> I think we need to check if the apply worker actually stopped, in
> addition to this check. Sub->enabled being false doesn't guarantee
> that the apply worker is not working.
>
> ---
> + foreach_oid(relid, relids)
> + {
> ...
> + if (relstate != SUBREL_STATE_READY && relstate !=
> SUBREL_STATE_SYNCDONE)
> + {
> ...
> +
> + must_use_password = sub->passwordrequired &&
> !sub->ownersuperuser;
> + wrconn = walrcv_connect(SubscriptionConninfo(sub), true, true,
> + must_use_password, sub->name, &err);
>
> The function establishes connections for each relation. We should use
> the one connection for all relations.
>
> ---
> Please update psql's tab-completion for the new syntax.
>
> Regards,
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
>