Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()

From: Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()
Date: 2018-01-12 11:34:09
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi Simon, Pavan,

I tried benchmarking your patch. I ran pgbench on a 72-core machine with
a simple append-only script:

INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid,
:bid, :aid, :delta, CURRENT_TIMESTAMP);
---- the insert repeated 20 times total -------

I can see an increase in transaction throughput of about 5%. Please see
the attached graph 'append-only.png'
On the default tpcb-like benchmark, there is no meaningful change in
performance, as shown in 'tpcb.png'

Looking at the code, your changes seem reasonable to me. Some nitpicks:

XLogSegNoLowOrderMask -- this macro is not really needed,
XLogSegNoToSegID is enough (pg_resetwal should be changed to use the
latter, too).

The loop in findLastCheckpoint looks complex to me. It would be easier
to read with two loops, the outer one iterating over the segments and
the inner one iterating over the records.

>+    uint16        xl_walid;        /* lowest 16 bits of the WAL file
to which this
>+                                   record belongs */

I'd put a high-level description here, the low-level details are hidden
behind macros anyway. Something like this: /* identifier of the WAL
segment to which this record belongs. Used to detect stale WAL records
in a reused segment. */

>+     * Make sure the above heavily-contended spinlock and byte
positions are
>+     * on their own cache line. In particular, the RedoRecPtr and
full page

This comment in the definition of XLogCtlInsert mentions the spinlock
that is no more.

The signature of InstallXLogFileSegment looks overloaded now, not sure
how to simplify it. We could have two versions, one that looks for the
free slot and another that reuses the existing file. Also, it seems to
me that the lock doesn't have to be acquired inside this function, it
can be done by the caller.

Alexander Kuzmenkov
Postgres Professional:
The Russian Postgres Company

Attachment Content-Type Size
image/png 12.9 KB
image/png 15.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ildar Musin 2018-01-12 11:50:51 Re: General purpose hashing func in pgbench
Previous Message Marina Polyakova 2018-01-12 11:05:08 Re: master make check fails on Solaris 10