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: Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@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-21T16:17:12Z
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 Mon, 21 Apr 2025 at 15:38, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear members,
>
> Thanks for reporting the issue.
>
> > Right. We have wrongly assumed in that commit that the apply worker
> > will exit after an ERROR, but as shown by this case, the ERROR could
> > be silently handled. So, +1, for moving replication origin reset to
> > PG_CATCH in start_apply.
>
> I was an author of original commit, so let me take initiative. When I was working
> for 3f28b2fcac, I could not find path which ERROR is reported but worker can
> survive so that I added replorigin_reset() in apply_error_callback(). The reported
> case, however, the exception could be raised but the insert itself is committed.
> In this case worker can continue working.
>
> Attached patches have proposed changes. I did 1) meson test, 2) workloads provided
> in [1], and 3) manual tests done in original thread [2], and all of them could be
> passed. The version is 2 because of the self-reviewing.
>
> One note is that geterrlevel() is removed for HEAD patch but retained for PG16/PG17.
> The function is exported, and APIs cannot be changed in back branches.
>
> How do you feel?

I was able to reproduce the issue with the steps suggested by Shawn
and your patch fixes the issue. Your suggested changes look good. One
thought, do you feel we should include a test for this.

Regards,
Vignesh