On Wed, Feb 14, 2024 at 12:58 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, 7 Feb 2024 at 16:27, vignesh C <vignesh21@gmail.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.