On Thu, Jan 26, 2017 at 1:38 PM, Nikhil Sontakke <nikh...@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+fks...@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-a54551a62...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
