Hi Simon, Pavan,
I tried benchmarking your patch. I ran pgbench on a 72-core machine with
a simple append-only script:
BEGIN;
INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid,
:bid, :aid, :delta, CURRENT_TIMESTAMP);
---- the insert repeated 20 times total -------
END;
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: http://www.postgrespro.com
The Russian Postgres Company
