Horton

Re: Disabled logical replication origin session causes primary key errors

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Shawn McCoy <shawn.the.mccoy@gmail.com>, "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>, "drewwcallahan@gmail.com" <drewwcallahan@gmail.com>, "scott@meads.us" <scott@meads.us>
Date: 2025-04-22T13:37:19Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix an oversight in 3f28b2fcac.

  2. Don't advance origin during apply failure.

On Tue, 22 Apr 2025 at 16:31, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Vignesh,
>
> > Couple of minor suggestions in the test:
> > 1) I felt this is not required for this test as it has been verified many times:
> > +
> > +# Insert tuples and confirms replication works well
> > +$node_publisher->safe_psql('postgres', "INSERT INTO t1 VALUES (1);");
> > +
> > +$node_publisher->wait_for_catchup('regress_sub');
>
> I added it to ensure remote_lsn to the valid value, but yes it is not mandatory.
>
> > 2) How about we change:
> > +# Confirms the origin can be advanced
> > +ok( $node_subscriber->poll_query_until(
> > +               'postgres',
> > +               "SELECT remote_lsn > '$remote_lsn' FROM
> > pg_catalog.pg_replication_origin_status os, pg_catalog.pg_subscription
> > s WHERE os.external_id = 'pg_' || s.oid AND s.subname =
> > 'regress_sub'",
> > +               't')
> > +         or die "Timed out while waiting for replication origin to be
> > updated");
> >
> > to:
> > $node_publisher->wait_for_catchup('regress_sub');
> >
> > # Confirms the origin can be advanced
> > $result = $node_subscriber->safe_psql('postgres',
> > "SELECT remote_lsn > '$remote_lsn' FROM
> > pg_catalog.pg_replication_origin_status os, pg_catalog.pg_subscription
> > s WHERE os.external_id = 'pg_' || s.oid AND s.subname = 'regress_sub'"
> > );
> > is($result, 't',
> > 'remote_lsn has advanced for apply worker raising an exception');
> >
> > In that way, we need not wait on poll_query_until.
>
> I intentionally used poll_query_until(), but I have no strong opinions.
> Modified.
>
> PSA new patches.

Thanks for the updated patch. I’ve reviewed and verified the changes
across the HEAD, PG17, and PG16 branches, and everything is working as
expected. I have no further comments.

Regards,
Vignesh