Horton

Re: Speed up Clog Access by increasing CLOG buffers

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Simon Riggs <simon@2ndquadrant.com>, Jesper Pedersen <jesper.pedersen@redhat.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2015-12-18T16:28:41Z
Lists: pgsql-hackers 
On Fri, Dec 18, 2015 at 1:16 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> 1. At scale factor 300, there is gain of 11% at 128-client count and
> 27% at 256 client count with Patch-1. At 4 clients, the performance with
> Patch is 0.6% less (which might be a run-to-run variation or there could
> be a small regression, but I think it is too less to be bothered about)
>
> 2. At scale factor 1000, there is no visible difference and there is some
> at lower client count there is a <1% regression which could be due to
> I/O bound nature of test.
>
> 3. On these runs, Patch-2 is mostly always worse than Patch-1, but
> the difference between them is not significant.

Hmm, that's interesting.  So the slots don't help.  I was concerned
that with only a single slot, you might have things moving quickly
until you hit the point where you switch over to the next clog
segment, and then you get a bad stall.  It sounds like that either
doesn't happen in practice, or more likely it does happen but the
extra slot doesn't eliminate the stall because there's I/O at that
point.  Either way, it sounds like we can forget the slots idea for
now.

>> Some random comments:
>>
>> - TransactionGroupUpdateXidStatus could do just as well without
>> add_proc_to_group.  You could just say if (group_no >= NUM_GROUPS)
>> break; instead.  Also, I think you could combine the two if statements
>> inside the loop.  if (nextidx != INVALID_PGPROCNO &&
>> ProcGlobal->allProcs[nextidx].clogPage == proc->clogPage) break; or
>> something like that.
>>
>> - memberXid and memberXidstatus are terrible names.  Member of what?
>
> How about changing them to clogGroupMemberXid and
> clogGroupMemberXidStatus?

What we've currently got for group XID clearing for the ProcArray is
clearXid, nextClearXidElem, and backendLatestXid.  We should try to
make these things consistent.  Maybe rename those to
procArrayGroupMember, procArrayGroupNext, procArrayGroupXid and then
start all of these identifiers with clogGroup as you propose.

>> That's going to be clear as mud to the next person looking at the
>> definitiono f PGPROC.
>
> I understand that you don't like the naming convention, but using
> such harsh language could sometimes hurt others.

Sorry.  If I am slightly frustrated here I think it is because this
same point has been raised about three times now, by me and also by
Andres, just with respect to this particular technique, and also on
other patches.  But you are right - that is no excuse for being rude.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Use group updates when setting transaction status in clog.

  2. Improve 64bit atomics support.

  3. Add ProcArrayGroupUpdate wait event.

  4. Make the different Unix-y semaphore implementations ABI-compatible.

  5. Fix broken ALTER INDEX documentation

  6. Code and docs review for commit 3187d6de0e5a9e805b27c48437897e8c39071d45.

  7. Partition the freelist for shared dynahash tables.

  8. Correct StartupSUBTRANS for page wraparound

  9. Make idle backends exit if the postmaster dies.

  10. contrib/sslinfo: add ssl_extension_info SRF

  11. Reduce ProcArrayLock contention by removing backends in batches.

  12. Fix `make installcheck` for serializable transactions.

  13. Lockless StrategyGetBuffer clock sweep hot path.

  14. Reduce sinval synchronization overhead.