Re: Why is subscription/t/031_column_list.pl failing so much?

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Why is subscription/t/031_column_list.pl failing so much?
Date: 2024-02-15 03:11:59
Message-ID: CAA4eK1KEmE3jxUV83naZKFmXYMi6Dn1mNpzBZRftqCWJ=YLUzw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 14, 2024 at 12:58 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, 7 Feb 2024 at 16:27, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > I was able to reproduce the issue consistently with the changes shared
> > by Tom Lane at [1].
> > I have made changes to change ALTER SUBSCRIPTION to DROP+CREATE
> > SUBSCRIPTION and verified that the test has passed consistently for
> > >50 runs that I ran. Also the test execution time increased for this
> > case is very negligibly:
> > Without patch: 7.991 seconds
> > With test change patch: 8.121 seconds
> >
> > The test changes for the same are attached.
>
> Alternative, this could also be fixed like the changes proposed by
> Amit at [1]. In this case we ignore publications that are not found
> for the purpose of computing RelSyncEntry attributes. We won't mark
> such an entry as valid till all the publications are loaded without
> anything missing. This means we won't publish operations on tables
> corresponding to that publication till we found such a publication and
> that seems okay.
>
> Tomas had raised a performance issue forcing us to reload it for every
> replicated change/row in case the publications are invalid at [2].
>

Did you measure the performance impact? I think it should impact the
performance only when there is a dropped/non-existent publication as
per the subscription definition. Also, in the same email[2], Tomas
raised another concern that in such cases it is better to break the
replication.

>
How
> about keeping the default option as it is and providing a new option
> skip_not_exist_publication while creating/altering a subscription. In
> this case if skip_not_exist_publication is specified we will ignore
> the case if publication is not present and publications will be kept
> in invalid and get validated later.
>

I don't know if this deserves a separate option or not but I think it
is better to discuss this in a separate thread. To resolve the BF
failure, I still feel, we should just recreate the subscription. This
is a pre-existing problem and we can track it via a separate patch
with a test case targetting such failures.

> The attached patch has the changes for the same. Thoughts?
>
> [1] - https://www.postgresql.org/message-id/CAA4eK1%2BT-ETXeRM4DHWzGxBpKafLCp__5bPA_QZfFQp7-0wj4Q%40mail.gmail.com
> [2] - https://www.postgresql.org/message-id/dc08add3-10a8-738b-983a-191c7406707b%40enterprisedb.com
>

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2024-02-15 03:16:41 Re: make add_paths_to_append_rel aware of startup cost
Previous Message vignesh C 2024-02-15 03:06:39 Re: Can we include capturing logs of pgdata/pg_upgrade_output.d/*/log in buildfarm