From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Amit Kapila <amit.kapila16@gmail.com>
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION
Date: Mon, 4 Dec 2023 13:00:27 +0100
Message-ID: <c4029baf-693d-4bb5-7c57-5bfcdc5572ff@enterprisedb.com> (raw)
In-Reply-To: <CAA4eK1KKgA+j4z2XYkAZh5cEzAUEdKw0-u96bu5a9PBbZumS7Q@mail.gmail.com>
References: <d291bb50-12c4-e8af-2af2-7bb9bb4d8e3e@enterprisedb.com>
<CAA4eK1LHD3ZHqyK2GcSw=fqCPr26yP2GL9niQbY8ems68D1Z9A@mail.gmail.com>
<8948e4a2-fec7-8a61-9ace-2537cee3ef6f@enterprisedb.com>
<CAA4eK1Kj6THmgXZ98d8rxAJa+V1KGS79KgqVve5i0Oyn+=tbMw@mail.gmail.com>
<69e9a1af-a315-8b05-0674-2cc53bf27682@enterprisedb.com>
<CAA4eK1L4wWiFnhDYhGMv-M4GTR6Xg13xoaSjhUm+YzFaP3nxUQ@mail.gmail.com>
<9afab261-94a7-3e58-f0fb-040e45f42aa5@enterprisedb.com>
<CANhcyEWtf6h5Ati69nbPoFXvFjfFsfrCekajWC=7JVnD0=cEfw@mail.gmail.com>
<CANhcyEWxjwHXka+A+9PLJqpvEa5fZXD5Du1cW1QqzAxjdBhrmg@mail.gmail.com>
<CAA4eK1KgyHAvDOPQgUdehSwUqDr0WxWtjxR5BHOMYkYaD2QZmA@mail.gmail.com>
<CANhcyEXKW7LjPrBBc_ttxxeFrJBQQ=GbGCfZKcy0-6_8ejnJcg@mail.gmail.com>
<CAA4eK1KKgA+j4z2XYkAZh5cEzAUEdKw0-u96bu5a9PBbZumS7Q@mail.gmail.com>
On 12/4/23 12:37, Amit Kapila wrote:
> On Sat, Dec 2, 2023 at 9:52 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>>
>>> thread. I think you can compare the timing of regression tests in
>>> subscription, with and without the patch to show there is no
>>> regression. And probably some tests with a large number of tables for
>>> sync with very little data.
>>
>> I have tested the regression test timings for subscription with and
>> without patch. I also did the timing test for sync of subscription
>> with the publisher for 100 and 1000 tables respectively.
>> I have attached the test script and results of the timing test are as follows:
>>
>> Time taken for test to run in Linux VM
>> Summary | Subscription Test (sec)
>> | 100 tables in pub and Sub (sec) | 1000 tables in pub and Sub
>> (sec)
>> Without patch Release | 95.564
>> | 7.877 | 58.919
>> With patch Release | 96.513
>> | 6.533 | 45.807
>>
>> Time Taken for test to run in another Linux VM
>> Summary | Subscription Test (sec)
>> | 100 tables in pub and Sub (sec) | 1000 tables in pub and Sub
>> (sec)
>> Without patch Release | 109.8145
>> | 6.4675 | 83.001
>> With patch Release | 113.162
>> | 7.947 | 87.113
>>
>
> So, on some machines, it may increase the test timing although not too
> much. I think the reason is probably doing the work in multiple
> transactions for multiple relations. I am wondering that instead of
> committing and starting a new transaction before
> wait_for_relation_state_change(), what if we do it inside that
> function just before we decide to wait? It is quite possible that in
> many cases we don't need any wait at all.
>
I'm not sure what you mean by "do it". What should the function do?
As for the test results, I very much doubt the differences are not
caused simply by random timing variations, or something like that. And I
don't understand what "Performance Machine Linux" is, considering those
timings are slower than the other two machines.
Also, even if it was a bit slower, does it really matter? I mean, the
current code is wrong, can lead to infinite duration if it happens to
hit the deadlock. And it's a one-time action, I don't think it's a very
sensitive in terms of performance.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@enterprisedb.com, amit.kapila16@gmail.com, shlok.kyal.oss@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: undetected deadlock in ALTER SUBSCRIPTION ... REFRESH PUBLICATION
In-Reply-To: <c4029baf-693d-4bb5-7c57-5bfcdc5572ff@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox