Re: Transactions involving multiple postgres foreign servers, take 2

On Fri, 21 Aug 2020 at 00:36, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
>
>
> On 2020/07/27 15:59, Masahiko Sawada wrote:
> > On Thu, 23 Jul 2020 at 22:51, Muhammad Usama <m(dot)usama(at)gmail(dot)com> wrote:
> >>
> >>
> >>
> >> On Wed, Jul 22, 2020 at 12:42 PM Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> >>>
> >>> On Sat, 18 Jul 2020 at 01:55, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2020/07/16 14:47, Masahiko Sawada wrote:
> >>>>> On Tue, 14 Jul 2020 at 11:19, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2020/07/14 9:08, Masahiro Ikeda wrote:
> >>>>>>>> I've attached the latest version patches. I've incorporated the review
> >>>>>>>> comments I got so far and improved locking strategy.
> >>>>>>>
> >>>>>>> Thanks for updating the patch!
> >>>>>>
> >>>>>> +1
> >>>>>> I'm interested in these patches and now studying them. While checking
> >>>>>> the behaviors of the patched PostgreSQL, I got three comments.
> >>>>>
> >>>>> Thank you for testing this patch!
> >>>>>
> >>>>>>
> >>>>>> 1. We can access to the foreign table even during recovery in the HEAD.
> >>>>>> But in the patched version, when I did that, I got the following error.
> >>>>>> Is this intentional?
> >>>>>>
> >>>>>> ERROR: cannot assign TransactionIds during recovery
> >>>>>
> >>>>> No, it should be fixed. I'm going to fix this by not collecting
> >>>>> participants for atomic commit during recovery.
> >>>>
> >>>> Thanks for trying to fix the issues!
> >>>>
> >>>> I'd like to report one more issue. When I started new transaction
> >>>> in the local server, executed INSERT in the remote server via
> >>>> postgres_fdw and then quit psql, I got the following assertion failure.
> >>>>
> >>>> TRAP: FailedAssertion("fdwxact", File: "fdwxact.c", Line: 1570)
> >>>> 0 postgres 0x000000010d52f3c0 ExceptionalCondition + 160
> >>>> 1 postgres 0x000000010cefbc49 ForgetAllFdwXactParticipants + 313
> >>>> 2 postgres 0x000000010cefff14 AtProcExit_FdwXact + 20
> >>>> 3 postgres 0x000000010d313fe3 shmem_exit + 179
> >>>> 4 postgres 0x000000010d313e7a proc_exit_prepare + 122
> >>>> 5 postgres 0x000000010d313da3 proc_exit + 19
> >>>> 6 postgres 0x000000010d35112f PostgresMain + 3711
> >>>> 7 postgres 0x000000010d27bb3a BackendRun + 570
> >>>> 8 postgres 0x000000010d27af6b BackendStartup + 475
> >>>> 9 postgres 0x000000010d279ed1 ServerLoop + 593
> >>>> 10 postgres 0x000000010d277940 PostmasterMain + 6016
> >>>> 11 postgres 0x000000010d1597b9 main + 761
> >>>> 12 libdyld.dylib 0x00007fff7161e3d5 start + 1
> >>>> 13 ??? 0x0000000000000003 0x0 + 3
> >>>>
> >>>
> >>> Thank you for reporting the issue!
> >>>
> >>> I've attached the latest version patch that incorporated all comments
> >>> I got so far. I've removed the patch adding the 'prefer' mode of
> >>> foreign_twophase_commit to keep the patch set simple.
> >>
> >>
> >> I have started to review the patchset. Just a quick comment.
> >>
> >> Patch v24-0002-Support-atomic-commit-among-multiple-foreign-ser.patch
> >> contains changes (adding fdwxact includes) for
> >> src/backend/executor/nodeForeignscan.c, src/backend/executor/nodeModifyTable.c
> >> and src/backend/executor/execPartition.c files that doesn't seem to be
> >> required with the latest version.
> >
> > Thanks for your comment.
> >
> > Right. I've removed these changes on the local branch.
>
> The latest patches failed to be applied to the master branch. Could you rebase the patches?
>

Thank you for letting me know. I've attached the latest version patch set.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services