Re: [bug fix] prepared transaction might be lost when max_prepared_transactions is zero on the subscriber

On Fri, Aug 9, 2024 at 9:35 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Aug 8, 2024 at 6:08 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > >
> > > I think this is a general issue that can occur not only due to 2PC. IIUC, this
> > > problem should arise if any ERROR arises after setting the
> > > replorigin_session_origin_lsn but before the CommitTransactionCommand is
> > > completed. If so, I think we should register it for tablesync worker as well.
> > >
> >
> > As pointed out earlier, won't using PG_ENSURE_ERROR_CLEANUP() instead
> > of PG_CATCH() be enough?
>
> Yes, I think it should suffice. IIUC, we are going to change
> 'replorigin_session_origin_lsn' only in start_apply() and not before
> that, and thus ensuring its reset during any ERROR or FATAL in
> start_apply() is good enough.
>

Right, I also think so.

> And I guess we don't want this
> origin-reset to be called during regular shutdown, isn't it?
>

Agreed. OTOH, there was no harm even if such a reset function is invoked.

> But
> registering it through before_shmem_exit() will make the
> reset-function to be called during normal shutdown as well.
>

True and unless I am missing something we won't need it. I would like
to hear the opinions of Hou-San and Kuroda-San on the same.

> And to answer my previous question (as Hou-San also pointed out), we
> do need it in table-sync worker as well. So a change in start_apply
> will make sure the fix is valid for both apply and tablesync worker.
>

As table-sync workers can also apply transactions after the initial
copy, we need it for table-sync during its apply phase.

--
With Regards,
Amit Kapila.