Re: Transactions involving multiple postgres foreign servers, take 2

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
Best regards
Muhammad Usama

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