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: Ajin Cherian <itsajin(at)gmail(dot)com>, "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-09-27 20:35:35
Message-ID: CAA36mso9RMn+EkbsBS=zzEs7JoOPLnxJR7YWSZ0kA1c0iuF9_g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Hayato,

Thank you for both rounds. The attached v10 answers all four points. It
also
carries the two fixes from your previous review, so it replaces v9.

In short:

- The tables are locked root first now, so naming a partition before its
root
no longer deadlocks.
- The TRUNCATE privilege is checked again under AccessExclusiveLock.
- Truncating the partitions of a subscribed partitioned table is what a
replicated TRUNCATE already does, so nothing extra is lost. Documented.
- REFRESH TABLE now runs the origin = none copy check, for the tables it
re-seeds.

From your earlier review:

> We may have to re-order relids to be the partition root first

Done. I reproduced it without gdb, by holding the partition in a prepared
transaction:

REFRESH TABLE test_c, test_p -- has test_c, wants test_p
INSERT INTO test_p -- has test_p, wants test_c
ERROR: deadlock detected

SortRefreshTableRelations() sorts the named tables by how many partitioned
ancestors each one has, which puts every root ahead of its partitions.
Unrelated tables keep the order given. The reproducer is clean afterwards.

> IIUC the check should be done after acquiring the AccessExclusive

Right. objectNamesToOids() takes only AccessShareLock, deliberately, so the
privilege can go away after the first check. TRUNCATE is safe because its
check sits in a RangeVarGetRelidExtended() callback that is re-run with the
lock held. This command has no callback, so it repeats the check itself.

From this review:

> In this case, REFRESH TABLE tab command truncate partition leaves. Data in
> tab_part1 could be re-synced but tuples in tab_part2 were gone. Is it OK?

It is not new. I ran your schema twice:

local-only rows in tab_part2 surviving, out of 5
publisher TRUNCATE : 0
REFRESH TABLE : 0

apply_handle_truncate() expands a partitioned target the same way, so a
TRUNCATE on the publisher already empties tab_part2. The docs now say so.

> How about separating the patch into two parts, 0001 is for normal tables
> (reject for partition root) and 0002 adds the partition handling?

Rejecting partitioned roots removes only about fifty lines. It does not
remove the ancestor locking or the ancestor half of the cross-subscription
check, which are needed when a leaf is the subscribed table, as it is
whenever
publish_via_partition_root is off. Both problems you found here were in
that
code, so it would stay in 0001 and 0002 would add the easy part. I would
rather keep one patch, but I will split it if you prefer.

> Can you evaluate the combination? I'm worry because initial sync does not
> take care the origin of tuples.

thank you. With C -> B -> A and A on origin = none, the same
copy made CREATE SUBSCRIPTION warn and REFRESH TABLE stay silent.
check_publications_origin_tables() skips the tables a subscription already
has, because those come through WAL where origins are known, and this
command
is the one thing that puts such a table back through a copy. v10 runs the
check and passes the subscription's other tables as the skip list, so it
warns
for a table another subscription writes and stays quiet otherwise.

check-world passes.

Best regards,
Cagri

On Fri, Sep 25, 2026 at 11:44 AM Hayato Kuroda (Fujitsu) <
kuroda(dot)hayato(at)fujitsu(dot)com> wrote:

> Dear Cagri,
>
> While thinking this feature again, I had two points:
>
> 01.
> Assuming the below schema definition, which only the subscriber has a
> partition:
>
> ```
> pub - sub
> tab tab
> - tab_part1 // stores data from pub
> - tab_part2 // stores data generated locally
> ```
>
> In this case, REFRESH TABLE tab command truncate partition leaves. Data in
> tab_part1 could be re-synced but tuples in tab_part2 were gone. Is it OK?
>
> I felt several issues have been reported for the partition table, and this
> trend
> would continue. How about separating the patch into two parts, 0001 is for
> normal
> tables (reject for partition root) and 0002 adds the partition handling?
>
> 02.
> Logical replication allowed to construct two(three)-way replication with
> the
> subscription parameter origin=none. Can you evaluate the combination? I'm
> worry
> because initial sync does not take care the origin of tuples.
>
> Best regards,
> Hayato Kuroda
> FUJITSU LIMITED
>

Attachment Content-Type Size
v10-0001-refresh-table.patch application/octet-stream 91.3 KB
v9-0001-refresh-table.patch application/octet-stream 84.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Scott Ray 2026-09-27 20:58:56 Re: pg_xmin_horizon: a system view of everything pinning the xmin horizon
Previous Message Scott Ray 2026-09-27 19:20:02 Re: pg_xmin_horizon: a system view of everything pinning the xmin horizon