Re: [HACKERS] Speedup twophase transactions

On Thu, Jan 26, 2017 at 1:38 PM, Nikhil Sontakke
<nikhils@2ndquadrant.com> wrote:
>> We should really try to do things right now, or we'll never come back
>> to it. 9.3 (if my memory does not fail me?) has reduced the time to do
>> promotion by removing the need of the end-of-recovery checkpoint,
>> while I agree that there should not be that many 2PC transactions at
>> this point, if there are for a reason or another, the time it takes to
>> complete promotion would be impacted. So let's refactor
>> PrescanPreparedTransactions() so as it is able to handle 2PC data from
>> a buffer extracted by XlogReadTwoPhaseData(), and we are good to go.
>
> Not quite. If we modify PrescanPreparedTransactions(), we also need to
> make RecoverPreparedTransactions() and
> StandbyRecoverPreparedTransactions() handle 2PC data via
> XlogReadTwoPhaseData().

Ah, right for both, even for RecoverPreparedTransactions() that
happens at the end of recovery. Thanks for noticing. The patch
mentions that as well:
+ *    * At the end of recovery we move all known prepared transactions to disk.
+ *      This allows RecoverPreparedTransactions() and
+ *      StandbyRecoverPreparedTransactions() to do their work.
I need some strong coffee..

>> +       KnownPreparedRecreateFiles(checkPoint.redo);
>>         RecoveryRestartPoint(&checkPoint);
>> Looking again at this code, I think that this is incorrect. The
>> checkpointer should be in charge of doing this work and not the
>> startup process, so this should go into CheckpointTwoPhase() instead.
>
> I don't see a function by the above name in the code?

I look at this patch from you and that's present for me:
https://www.postgresql.org/message-id/CAMGcDxf8Bn9ZPBBJZba9wiyQq-Qk5uqq=VjoMnRnW5s+fKST3w@mail.gmail.com
If I look as well at the last version of Stas it is here:
https://www.postgresql.org/message-id/BECC988A-DB74-48D5-B5D5-A54551A6242A@postgrespro.ru

As this change:
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9573,6 +9573,7 @@ xlog_redo(XLogReaderState *record)                    (errmsg("unexpected timeline ID %u (should
be%u)
 
in checkpoint record",                            checkPoint.ThisTimeLineID, ThisTimeLineID)));

+        KnownPreparedRecreateFiles(checkPoint.redo);        RecoveryRestartPoint(&checkPoint);    }
And actually, when a XLOG_CHECKPOINT_SHUTDOWN record is taken, 2PC
files are not flushed to disk with this patch. This is a problem as a
new restart point is created... Having the flush in CheckpointTwoPhase
really makes the most sense.
-- 
Michael