Greetings, * Chapman Flack (c...@anastigmatix.net) wrote: > On 03/18/18 19:28, Daniel Gustafsson wrote: > > It seems expensive to regex over BLCKSZ, but it’s probably the safest option > > and it’s not a performance critical codepath. Feel free to whack the test > > patch over the head with the above diff. > > Both patches in a single email for cfbot's enjoyment, coming right up.
Thanks for this. I'm also of the opinion, having read through the
thread, that it was an unintentional side-effect to have the headers in
the otherwise-zero'd end of the WAL segment and that re-zero'ing the end
makes sense and while it's a bit of extra time, it's not on a
performance-critical path given this is only happening on a less-busy
system to begin with.
I'm mildly disappointed to see the gzip has only a 2x improvement with
the change, but that's certainly not this patch's fault.
Reviewing the patch itself..
> .travis.yml | 47
> ++++++++++++++++++++++++++++++++++++
... not something that I think we're going to add into the main tree.
> diff --git a/src/backend/access/transam/xlog.c
> b/src/backend/access/transam/xlog.c
> index 47a6c4d..a91ec7b 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -1556,7 +1556,16 @@ CopyXLogRecordToWAL(int write_len, bool isLogSwitch,
> XLogRecData *rdata,
> {
> /* initialize the next page (if not initialized
> already) */
> WALInsertLockUpdateInsertingAt(CurrPos);
> - AdvanceXLInsertBuffer(CurrPos, false);
> + /*
> + * Fields in the page header preinitialized by
> AdvanceXLInsertBuffer
> + * to nonconstant values reduce the compressibility of
> WAL segments,
> + * and aren't needed in the freespace following a
> switch record.
> + * Re-zero that header area. This is not
> performance-critical, as
> + * the more empty pages there are for this loop to
> touch, the less
> + * busy the system is.
> + */
> + currpos = GetXLogBuffer(CurrPos);
> + MemSet(currpos, 0, SizeOfXLogShortPHD);
> CurrPos += XLOG_BLCKSZ;
> }
> }
AdvanceXLInsertBuffer() does quite a bit, so I'm a bit surprised to see
this simply removing that call, you're confident there's nothing done
which still needs doing..?
Thanks!
Stephen
signature.asc
Description: PGP signature
