On Wed, Jan 25, 2017 at 11:55 PM, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote: >> We are talking about the recovery/promote code path. Specifically this >> call to KnownPreparedRecreateFiles() in PrescanPreparedTransactions(). >> >> We write the files to disk and they get immediately read up in the >> following code. We could not write the files to disk and read >> KnownPreparedList in the code path that follows as well as elsewhere. > > Thinking more on this. > > The only optimization that's really remaining is handling of prepared > transactions that have not been committed or will linger around for > long. The short lived 2PC transactions have been optimized already via > this patch. > > The question remains whether saving off a few fsyncs/reads for these > long-lived prepared transactions is worth the additional code churn. > Even if we add code to go through the KnownPreparedList, we still will > have to go through the other on-disk 2PC transactions anyways. So, > maybe not.
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. --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -9573,6 +9573,15 @@ xlog_redo(XLogReaderState *record) (errmsg("unexpected timeline ID %u (should be %u) in checkpoint record", checkPoint.ThisTimeLineID, ThisTimeLineID))); + + /* + * Move prepared transactions, if any, from KnownPreparedList to files. + * It is possible to skip this step and teach subsequent code about + * KnownPreparedList, but PrescanPreparedTransactions() happens once + * during end of recovery or on promote, so probably it isn't worth + * the additional code. + */ + 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. At the end, we should be able to just live without KnownPreparedRecreateFiles() and just rip it off from the patch. -- Michael -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers