| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Cagri Biroglu <cagri(dot)biroglu(at)adyen(dot)com> |
| Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, 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-11-02 11:43:01 |
| Message-ID: | CANhcyEURLDfaC1zuS=8uRYqcXfFyS+x_VP9P7sYVPk3r2XpvZg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 2 Nov 2026 at 01:35, Cagri Biroglu <cagri(dot)biroglu(at)adyen(dot)com> wrote:
>
> Hi Shlok ,
>
> thank you , fixed in v13, attached.
>
> REFRESH TABLE held pg_subscription_rel while it waited for a lock on a user
> relation. It now takes every relation lock first and opens the catalogue
> afterwards, which is the order AlterSubscription_refresh() already uses, so
> the cycle cannot form. The lock level was not the problem: RowExclusiveLock
> conflicts with the AccessExclusiveLock that command wants there just as
> surely.
>
> 039 asserts it directly -- while REFRESH TABLE waits on a relation lock it
> must hold no lock on pg_subscription_rel -- and then runs your two commands
> concurrently in that window. Four of those five checks fail if the change is
> reverted, one with "deadlock detected".
>
> The reproduction is attached as poc_refresh_publication_deadlock.sh: your
> recipe, without gdb, so it can run unattended. A prepared transaction holds a
> second table named in the REFRESH TABLE, which parks the command in its first
> pass exactly where your breakpoint was. Can you please confirm ?
>
> B opens pg_subscription_rel, resolves t1, waits on t_block
> A takes AccessShareLock on t1, then waits on pg_subscription_rel
> X commits, releasing t_block
> B asks for AccessExclusiveLock on t1, which A holds -> deadlock detected
Hi Cagri,
I tested and agree that poc_refresh_publication_deadlock.sh can
reproduce the issue in a deterministic way.
But I also noticed that test 039_refresh_table.pl is failing in my
local machine.
I have also confirmed that the change suggested by Song in [1]
addresses this failure.
Apart from that I have a doubt for v13-0001 patch:
Suppose we have a tablesync in a mid-sync state such as FINISHEDCOPY
or DATASYNC,
And we run ALTER SUBSCRIPTION ... REFRESH TABLE for such a table, and
due to some
reason publisher is down, the following code will drop the replication origin
corresponding to the tablesync:
+ foreach_oid(relid, originrelids)
+ {
+ ReplicationOriginNameForLogicalRep(sub->oid, relid, originname,
+ sizeof(originname));
+ replorigin_drop_by_name(originname, true, false);
+ }
But at the later stage, the REFRESH TABLE command will fail as publisher is down
and subscriber will not be able to connect to the publisher:
+ load_file("libpqwalreceiver", false);
+
+ must_use_password = sub->passwordrequired && !sub->ownersuperuser;
+ wrconn = walrcv_connect(SubscriptionConninfo(sub), true, true,
+ must_use_password, sub->name, &err);
+ if (!wrconn)
+ ereport(ERROR,
+ errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("subscription \"%s\" could not connect to the publisher: %s",
+ sub->name, err));
+
Since the command failed, the transactional block will rollback: tablesync will
be setback to FINISHEDCOPY/DATASYNC state, the TRUNCATE command will rollback.
But the replication origin status (in pg_replication_origin_status)
will remain dropped.
Is this expected? Could this lead to any unexpected behaviour?
I am able to reproduce the scenario, but I am not able to find any
unexpected behaviour
during my testing. But I am raising the comment to be on the safe side.
Thanks,
Shlok Kyal
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Pavel Stehule | 2026-11-02 11:52:34 | Re: PL/pgSQL: apply the too_many_rows extra check to EXECUTE ... INTO |
| Previous Message | Peter Eisentraut | 2026-11-02 11:37:27 | Re: [PATCH] Provide support for trailing commas |