Re: WALInsertLock tuning

I assume this was addressed with this commit:
commit 465883b0a2b4236ba6b31b648a9eabef3b7cdddbAuthor: Simon Riggs <simon@2ndQuadrant.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
outinvalidation 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@gmail.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +