> On 07 Sep 2016, at 11:07, Stas Kelvich <s.kelvich@postgrespro.ru> wrote:
>
>> On 07 Sep 2016, at 03:09, Michael Paquier <michael.paquier@gmail.com> wrote:
>>
>>>> On 06 Sep 2016, at 12:03, Michael Paquier <michael.paquier@gmail.com> wrote:
>>>>
>>>> On Tue, Sep 6, 2016 at 5:58 PM, Stas Kelvich <s.kelvich@postgrespro.ru> wrote:
>>>>> Oh, I was preparing new version of patch, after fresh look on it. Probably, I should
>>>>> said that in this topic. I’ve found a bug in sub transaction handling and now working
>>>>> on fix.
>>>>
>>>> What's the problem actually?
>>>
>>> Handling of xids_p array in PrescanPreparedTransactions() is wrong for prepared tx's in memory.
>>> Also I want to double-check and add comments to RecoveryInProgress() checks in FinishGXact.
>>>
>>> I’ll post reworked patch in several days.
>>
>> Could you use as a base the version I just sent yesterday then? I
>> noticed many mistakes in the comments, for example many s/it's/its/
>> and did a couple of adjustments around the code, the goto next_file
>> was particularly ugly. That will be that much work not do to again
>> later.
>
> Yes. Already merged branch with your version.
Here is updated version of patch.
Looking through old version i’ve noted few things that were bothering me:
* In case of crash replay PREPARE redo accesses SUBTRANS, but StartupSUBTRANS() isn’t called yet
in StartupXLOG().
* Several functions in twophase.c have to walk through both PGPROC and pg_twophase directory.
Now I slightly changed order of things in StartupXLOG() so twophase state loaded from from file and
StartupSUBTRANS called before actual recovery starts. So in all other functions we can be sure that
file were already loaded in memory.
Also since 2pc transactions now are dumped to files only on checkpoint, we can get rid of
PrescanPreparedTransactions() — all necessary info can reside in checkpoint itself. I’ve changed
behaviour of oldestActiveXid write in checkpoint and that’s probably discussable topic, but ISTM
that simplifies a lot recovery logic in both twophase.c and xlog.c. More comments on that in patch itself.
--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company