Re: Per-table resync for logical replication subscriptions

From: Cagri Biroglu <cagri(dot)biroglu(at)adyen(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: "smithpb2250(at)gmail(dot)com" <smithpb2250(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: Per-table resync for logical replication subscriptions
Date: 2026-08-15 07:38:21
Message-ID: CAA36msrNHhDA9n+LaW_VLdbh5y=R_w_Q5wyxMyM4ETuuC6t=Xw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Hayato,

Thank you for testing, and especially for the reproducer.

One note on versions first, to save you looking for something that is not
there: v3 is the last version I posted, and three reviews landed on it,
including yours. v4 folds in all of them at once, so it is a larger step
than usual. The v3 to v4 delta is:

- drop the tablesync slots last, over a single publisher connection
- reject the command when two_phase is enabled (your 01)
- take AccessExclusiveLock when the relations are resolved
- use ExecuteTruncateGuts() with resolved OIDs, plus an explicit
TRUNCATE privilege check, which that function does not do
- wait for all worker types, not just the apply worker (your 02)
- reject relations another subscription also populates (your 03)
- stop following inheritance children, which lost their rows
- hint when a partitioned root is not tracked by the subscription
- psql tab completion, and documentation, which v3 had none of

Taking your points in turn.

> 01. two_phase

Already in v4, added for Masahiko's review with the same reasoning as
yours. There is no "&& opts.copy_data" term, since REFRESH TABLE always
copies and so has no exception to carve out. I used
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE rather than ERRCODE_SYNTAX_ERROR,
to match the adjacent state checks such as "cannot disable two_phase when
prepared transactions exist"; nothing is syntactically wrong here. Happy
to switch if you prefer it to match REFRESH PUBLICATION exactly.

On the prepared transaction stall: with this guard it is unreachable,
because subscription gxacts only exist once two_phase is ENABLED, and
two_phase cannot be turned off while they exist. Note this means your
reproducer stops early on v4, since both subscriptions use two_phase = on;
I removed that to reach point 03.

> 02. check the leader worker too, and drop the stopping code

Agreed and adopted, and this closed a hole I had not seen. The check I had
written for first review covered only WORKERTYPE_APPLY, and a tablesync
worker outlives its apply worker: on a 1 GB table the apply worker is gone
within a second of DISABLE while the tablesync worker keeps copying for
another 12. So that check could pass while a tablesync worker still held
the relation.

It now uses logicalrep_workers_find(subid, false, true) and the
logicalrep_worker_stop() loop is gone. I chose only_running = false, as
DROP SUBSCRIPTION does, because a worker that is in_use but has not
attached yet is the dangerous one: it is about to read the relation state
we are rewriting.

> 03. TRUNCATE happens but srsubstate cannot be updated

Confirmed. Your case with two_phase removed:

before: foo_1 has 9 rows, a=2,4,6,8 from sub2 and a=15,16,18,19,20
routed by sub1
after REFRESH TABLE foo on sub1, then ENABLE: foo_1 has 5 rows

sub2's four rows were destroyed and never copied back, because sub2's
state stayed 'r'.

Fixing it, I found the problem is more general than partitioning. The same
loss happens with no partitions at all, when two subscriptions carry
disjoint row filters on one table:

CREATE PUBLICATION p_lo FOR TABLE t WHERE (a < 10);
CREATE PUBLICATION p_hi FOR TABLE t WHERE (a >= 10);
-- s_lo and s_hi on the subscriber, 20 rows
ALTER SUBSCRIPTION s_hi DISABLE;
ALTER SUBSCRIPTION s_hi REFRESH TABLE t; -- truncates all 20
ALTER SUBSCRIPTION s_hi ENABLE;
-- t has 11 rows; s_lo's 9 are gone

So the rule the patch was missing is that a relation must not be discarded
unless the state of whatever will refill it is reset too. A relation is
refilled by any subscription that tracks it directly, and, if it is a
partition, by any subscription that tracks a partitioned ancestor and
routes rows into it. v4 checks that before touching anything, so the
command stays all-or-nothing, and rejects instead of losing data:

ERROR: table "t" is also part of the subscription "s_lo"
ERROR: table "foo_1" is also part of the subscription "sub2"
ERROR: table "pp_1" is a partition of "pp", which is part of the
subscription "s_root"

The last is the reverse of your case: naming the partition while another
subscription tracks the root.

I chose to refuse rather than reset the other subscription's relations
too. Doing that properly would mean requiring the other subscription to be
disabled as well, resetting catalog rows belonging to it, and dropping its
tablesync slots over its own connection, all from a command naming a
different subscription. That felt like a bigger decision than I should
make alone, so v4 turns silent data loss into a clear error. If you think
the cross-subscription reset is the right end state, I am glad to write it.

Six regression checks cover the three shapes, including that the data is
still intact after the command is rejected. Against the previous code five
of them fail, among them both "no rows lost" assertions.

Best regards,
Cagri Biroglu

On Fri, Aug 14, 2026 at 12:18 PM Hayato Kuroda (Fujitsu) <
kuroda(dot)hayato(at)fujitsu(dot)com> wrote:

> Dear Cagri,
>
> Thanks for updating the patch. I read and tested your patch. Here are my
> comments.
>
> 01.
> According to the REFRESH PUBLICATION command, we must prohibit to
> synchronize
> tables if the two_phase is enabled, but REFRESH TABLE seems to bypass the
> restriction.
> Also, if there are prepared transactions done by the subscription and they
> modify
> target relations, the command would stuck forever - the apply worker won't
> start
> again. So should we have the same guard as the REFRESH PUBLICATION?
>
> 02.
> You have already checked the case the existence of tablesync workers, but
> not for the leader worker. I feel we should ensure via
> logicalrep_workers_find()
> like ALTER SUBSCRIPTION SET (two_phase). Also, we can remove the part from
> AlterSubscription_refresh_table().
>
> 03.
> Regarding the partition table, I found the case that TRUNCATE happened but
> srsubstate cannot be updated. It's because ExecuteTruncate() truncates
> tables all
> child tables but AlterSubscription_refresh_table() updates tuples relid is
> exactly matched. Per my experiment, this can cause the issue if the
> different publications
> publish the root and child separately, and they are subscribed by the
> different subscription.
> See attached reproducer.
>
> Best regards,
> Hayato Kuroda
> FUJITSU LIMITED
>
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Imran Zaheer 2026-08-15 07:57:15 Failing assertion while taking a restartpoint during crash recovery
Previous Message Cagri Biroglu 2026-08-15 07:23:23 Re: Per-table resync for logical replication subscriptions