Thanks a lot for your edits, now that patch is much more cleaner. > Your comments say > > "In case of crash replay will move data from xlog to files, if that hasn't happened before." > > but I don't see that in code. Can you show me where that happens?
xact.c calls RecreateTwoPhaseFile in xact_redo() function (xact.c:5596)
So we've only optimized half the usage? We're still going to cause replication delays.
Sounds like we should be fixing both.
We can either
1) Skip fsyncing the RecreateTwoPhaseFile and then fsync during restartpoints
2) Copy the contents to shmem and then write them at restartpoint as we do for checkpoint
(preferred)
> On 09 Jan 2016, at 18:29, Simon Riggs <simon@2ndquadrant.com> wrote: > > Hmm, I was just preparing this for commit. > > Please have a look at my mild edits and extended comments.
One concern that come into my mind while reading updated patch is about creating extra bool field in GlobalTransactionData structure. While this improves readability, it also increases size of that structure and that size have impact on performance on systems with many cores (say like 60-80). Probably one byte will not make measurable difference, but I think it is good idea to keep GXact as small as possible. As far as I understand the same logic was behind split of PGPROC to PGPROC+PGXACT in 9.2 (comment in proc.h:166)
I think padding will negate the effects of the additional bool.
If we want to reduce the size of the array GIDSIZE is currently 200, but XA says maximum 128 bytes.
Anybody know why that is set to 200?
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
