Re: Transactions involving multiple postgres foreign servers, take 2

On Thu, 23 Jul 2020 at 22:51, Muhammad Usama <m.usama@gmail.com> wrote:
>
>
>
> On Wed, Jul 22, 2020 at 12:42 PM Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote:
>>
>> On Sat, 18 Jul 2020 at 01:55, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> >
>> >
>> >
>> > On 2020/07/16 14:47, Masahiko Sawada wrote:
>> > > On Tue, 14 Jul 2020 at 11:19, Fujii Masao <masao.fujii@oss.nttdata.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.

Regards,

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