Re: Transactions involving multiple postgres foreign servers, take 2

On 2020/07/27 15:59, Masahiko Sawada wrote:
> 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.

The latest patches failed to be applied to the master branch. Could you rebase the patches?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION