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-28 08:54:35
Message-ID: CAA36msqi1Up_-bd_iUKWgADD9szqG6Ot==TbDDNE1qiYjF6Hcg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello again ,

needed a rebase. v11 is attached.

Best regards,
Cagri

On Sun, Sep 27, 2026 at 10:35 PM Cagri Biroglu <cagri(dot)biroglu(at)adyen(dot)com>
wrote:

> 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
v11-0001-refresh-table.patch application/octet-stream 91.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jakub Wartak 2026-09-28 08:58:58 Re: enhancing pg_basebackup speeds up to ~23Gbps (small fixes + io_uring/Direct I/O)
Previous Message Bertrand Drouvot 2026-09-28 07:46:06 Re: Persist slot invalidations before publishing them