On 07/17/17 11:29, Michael Paquier wrote:
> FWIW, I would rather see any optimization done in
> AdvanceXLInsertBuffer() instead of seeing a second memset re-zeroing
> the WAL page header after its data has been initialized by
> AdvanceXLInsertBuffer() once.
Is that an aesthetic 'rather', or is there a technical advantage you
have in mind?
I also began by looking at how to stop AdvanceXLInsertBuffer()
initializing headers and taking locks when neither is needed.
But Heikki's just-rezero-them suggestion has a definite simplicity
advantage. It can be implemented entirely with a tight group of
lines added to CopyXLogRecordToWAL, as opposed to modifying
AdvanceXLInsertBuffer in a few distinct places, adding a parameter,
and changing its call sites.
There's a technical appeal to making the changes in AdvanceXLInsertBuffer
(who wants to do unnecessary initialization and locking?), but the amount
of unnecessary work that can be avoided is proportional to the number of
unused pages at switch time, meaning it is largest when the system
is least busy, and may be of little practical concern.
Moreover, optimizing AdvanceXLInsertBuffer would reveal one more
complication: some of the empty pages about to be written out may
have been initialized opportunistically in earlier calls to
AdvanceXLInsertBuffer, so those already have populated headers, and
would need rezeroing anyway. And not necessarily just an insignificant
few of them: if XLOGChooseNumBuffers chose the maximum, it could even
be all of them.
That might also be handled by yet another conditional within
AdvanceXLInsertBuffer. But with all of that in view, maybe it is
just simpler to have one loop in CopyXLogRecordToWAL simply zero them all,
and leave AdvanceXLInsertBuffer alone, so no complexity is added when it
is called from other sites that are arguably hotter.
Zeroing SizeOfXLogShortPHD bytes doesn't cost a whole lot.
-Chap