Re: Add an option to skip loading missing publication to avoid logical replication failure

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add an option to skip loading missing publication to avoid logical replication failure
Date: 2025-03-04 06:52:51
Message-ID: CALDaNm2Xkm1M-ik2RLJZ9rMhW2zW2GRLL6ePyZJbXcAjOVwzXg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 3 Mar 2025 at 16:41, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Mar 3, 2025 at 2:30 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Tue, 25 Feb 2025 at 15:32, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> > > The attached script has the script that was used for testing. Here the
> > > NUM_RECORDS count should be changed accordingly for each of the tests
> > > and while running the test with the patch change uncomment the drop
> > > publication command.
> >
> > I have done further analysis on the test and changed the test to
> > compare it better with HEAD. The execution time is in milliseconds.
> > Brach/records | 100 | 1000 | 10000 | 100000 | 1000000
> > Head | 10.43 | 15.86 | 64.44 | 550.56 | 8991.04
> > Patch | 11.35 | 17.26 | 73.50 | 640.21 | 10104.72
> > % diff | -8.82 | -8.85 | -14.08 | -16.28 | -12.38
> >
> > There is a performance degradation in the range of 8.8 to 16.2 percent.
> >
>
> - /* Validate the entry */
> - if (!entry->replicate_valid)
> + /*
> + * If the publication is invalid, check for updates.
> + * This optimization ensures that the next block, which queries the system
> + * tables and builds the relation entry, runs only if a new publication was
> + * created.
> + */
> + if (!publications_valid && data->publications)
> + {
> + bool skipped_pub = false;
> + List *publications;
> +
> + publications = LoadPublications(data->publication_names, &skipped_pub);
>
> The publications_valid flag indicates whether the publications cache
> is valid or not; the flag is set to false for any invalidation in the
> pg_publication catalog. I wonder that instead of using the same flag
> what if we use a separate publications_skipped flag? If that works,
> you don't even need to change the current location where we
> LoadPublications.

There is almost negligible dip with the above suggested way, the test
results for the same is given below(execution time is in milli
seconds):
Brach/records | 100 | 1000 | 10000 | 100000 | 1000000
Head | 10.25 | 15.85 | 65.53 | 569.15 | 9194.19
Patch | 10.25 | 15.84 | 65.91 | 571.75 | 9208.66
% diff | 0.00 | 0.06 | -0.58 | -0.46 | -0.16

There is a performance dip in the range of 0 to 0.58 percent.
The attached patch has the changes for the same. The test script used
is also attached.

Regards,
Vignesh

Attachment Content-Type Size
v5-0001-Fix-logical-replication-breakage-after.patch application/x-patch 4.4 KB
test_pg_recvlogical.sh text/x-sh 1.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2025-03-04 06:58:09 Re: Doc fix of aggressive vacuum threshold for multixact members storage
Previous Message Julien Tachoires 2025-03-04 06:16:19 Re: Allow table AMs to define their own reloptions