Re: WALInsertLock tuning

I assume this was addressed with this commit:

commit 465883b0a2b4236ba6b31b648a9eabef3b7cdddb
Author: Simon Riggs <simon(at)2ndQuadrant(dot)com>
Date: Tue Jun 28 22:58:17 2011 +0100

Introduce compact WAL record for the common case of commit (non-DDL).
XLOG_XACT_COMMIT_COMPACT leaves out invalidation messages and relfilenodes,
saving considerable space for the vast majority of transaction commits.
XLOG_XACT_COMMIT keeps same definition as XLOG_PAGE_MAGIC 0xD067 and earlier.

Leonardo Francalanci and Simon Riggs

---------------------------------------------------------------------------

Simon Riggs wrote:
> On Mon, Jun 6, 2011 at 11:25 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> > As to the question of whether it's safe, I think I'd agree that the
> > chances of this backfiring are pretty remote. ?I think that with the
> > zeroing they are exactly zero, because (now that we start XLOG
> > positions at 0/1) there is now way that xl_prev = {0, 0} can ever be
> > valid. ?Without the zeroing, well, it's remotely possible that xl_prev
> > could happen to appear valid and that xl_crc could happen to match...
> > but the chances are presumably quite remote. ?Just the chances of the
> > CRC matching should be around 2^-32. ?The chances of an xl_prev match
> > are harder to predict, because the matching values for CRCs should be
> > pretty much uniformly distributed, while xl_prev isn't random. ?But
> > even given that the chance of a match is should be very small, so in
> > practice there is likely no harm.
>
> And if such a thing did actually happen we would also need to have an
> accidentally correct value of all of the rest of the header values.
> And even if we did we would apply at most one junk WAL record. Then we
> are onto the next WAL record where we would need have to the same luck
> all over again.
>
> The probability of these occurrences is well below the acceptable
> threshold for other problems occurring.
>
> > It strikes me, though, that we
> > could probably get nearly all of the benefit of this patch by being
> > willing to zero the first sizeof(XLogRecord) bytes following a record,
> > but not the rest of the buffer. ?That would pretty much wipe out any
> > chance of an xl_prev match, I think, and would likely still get nearly
> > all of the performance benefit.
>
> Which adds something onto the path of every XlogInsert(), rather than
> once per page, so I'm a little hesitant to agree.
>
> If we did that, we would only need to write out an additional 12 bytes
> per WAL record, not the full sizeof(XLogRecord).
>
> But even so, I think its wasted effort.
>
> Measuring the benefit of a performance patch is normal, but I'm not
> proposing this as a risk trade-off. It's just a straight removal of
> multiple cycles from a hot code path. The exact benefit will depend
> upon whether the WALInsertLock is the hot lock, which it likely will
> be when other patches are applied.
>
> --
> ?Simon Riggs?????????????????? http://www.2ndQuadrant.com/
> ?PostgreSQL Development, 24x7 Support, Training & Services
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +