Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION

Shlok Kyal <shlok.kyal.oss@gmail.com>

From: Shlok Kyal <shlok.kyal.oss@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-12-07T05:51:29Z
Lists: pgsql-hackers

Attachments

Hi,

> I mean to commit the open transaction at the below place in
> wait_for_relation_state_change()
>
> wait_for_relation_state_change()
> {
> ...
> -- commit the xact
> WaitLatch();
> ...
> }
>
> Then start after the wait is over. This is just to test whether it
> improves the difference in regression test timing.

I tried the above approach and observed that the performance of this
approach is nearly same as the previous approach.

For Linux VM:
Summary    | Subscription | 100 tables in pub  | 1000 tables in pub
                    | Test (sec)     | and Sub (sec)       | and Sub (sec)
------------------------------------------------------------------------------
old patch     |  107.4545     |   6.911           | 77.918
alternate      |  108.3985    | 6.9835           | 78.111
approach

For Performance Machine:
Summary    | Subscription | 100 tables in pub  | 1000 tables in pub
                    | Test (sec)     | and Sub (sec)       | and Sub (sec)
------------------------------------------------------------------------------
old patch     |  115.922      |   6.7305         | 81.1525
alternate      |  115.8215   | 6.7685            | 81.2335
approach

I have attached the patch for this approach as 'alternate_approach.patch'.

Since the performance is the same, I think that the previous approach
is better. As in this approach we are using CommitTransactionCommand()
and StartTransactionCommand() inside a 'for loop'.

I also fixed the comment in previous approach and attached here as
'v2-0001-Deadlock-when-apply-worker-tablesync-worker-and-c.patch'

Thanks and Regards


Shlok Kyal

Commits

  1. Fix an undetected deadlock due to apply worker.