From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Kapila <amit.kapila16@gmail.com>
cc: shveta malik <shveta.malik@gmail.com>,
        "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
        "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
        "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: [bug fix] prepared transaction might be lost when max_prepared_transactions is zero on the subscriber
In-reply-to: <CAA4eK1KjgViaqQGR6897+20Z_zOARr4CrZr06jmc3Euq22453Q@mail.gmail.com>
References: <TYAPR01MB5692FAC23BE40C69DA8ED4AFF5B92@TYAPR01MB5692.jpnprd01.prod.outlook.com> <CAA4eK1J8j33OrTVGRgpw9eGYsFyyT=XyLJ2eC4DpzsoggYKTDg@mail.gmail.com> <TYAPR01MB5692AE79A51187517DF84A43F5BA2@TYAPR01MB5692.jpnprd01.prod.outlook.com> <CAJpy0uA_BMM_Rb-1a7SNubDJ0rhTya5fACAHa2SmzgdnWRf63Q@mail.gmail.com> <CAA4eK1JwxN8r4K+E1USuQkqoKUkTVnCZbh+fvQP7DP7buWheKQ@mail.gmail.com> <CAJpy0uAa3M9sC3GWTUA+YPTnYxEcorFt_7g6rXz05jNcAGdjgQ@mail.gmail.com> <CAA4eK1K=hFfrB1rW5Mbd5aTGBP_yo1nKb-0jdib842Z9bTz_Dw@mail.gmail.com> <CAJpy0uA+yBtU0kUKpioy9hvUXriEnnz871zP1dirK5ba2x7V4w@mail.gmail.com> <CAA4eK1KjgViaqQGR6897+20Z_zOARr4CrZr06jmc3Euq22453Q@mail.gmail.com>
Comments: In-reply-to Amit Kapila <amit.kapila16@gmail.com>
	message dated "Wed, 21 Aug 2024 15:27:46 +0530"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <3458726.1731007427.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 07 Nov 2024 14:23:47 -0500
Message-ID: <3458727.1731007427@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3458727.1731007427%40sss.pgh.pa.us>
Precedence: bulk

Amit Kapila <amit.kapila16@gmail.com> writes:
> On Tue, Aug 20, 2024 at 2:01=E2=80=AFPM shveta malik <shveta.malik@gmail=
.com> wrote:
>> Looks good to me. Please find backported patches attached.

> Pushed.

I came across this commit while preparing release notes, and I'm
worried about whether it doesn't create more problems than it solves.
The intent stated in the thread subject is to prevent an apply worker
from advancing past a prepared transaction if the subscriber side
doesn't permit prepared transactions.  However, it appears to me that
the committed patch doesn't permit an apply worker to advance past
any failing transaction whatsoever.  Was any thought given to how
a DBA would get out of such a situation and get replication flowing
again?  In the prepared-xact case, it's at least clear that you
could increase max_prepared_transactions and restart the subscriber
installation.  In the general case, it's not very obvious that you'd
even know what the problem is let alone have an easy way to fix it.

In other words: I thought the original design here was to
intentionally ignore apply errors and keep going, on the theory that
that was better than blocking replication altogether.  This commit
has reversed that decision, on the strength of little or no
discussion AFAICS.  Are we really ready to push this into minor
releases of stable branches?  Is it a good idea even on HEAD?

			regards, tom lane