On Fri, Aug 9, 2024 at 9:35 AM shveta malik <shveta.ma...@gmail.com> wrote: > > On Thu, Aug 8, 2024 at 6:08 PM Amit Kapila <amit.kapil...@gmail.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.
