On Sun, May 13, 2012 at 09:01:03PM +0100, Peter Geoghegan wrote:
> On 12 May 2012 01:37, Robert Haas <robertmh...@gmail.com> wrote:
> > Right.  It's not a new feature; it's a performance improvement.  We've
> > had group commit for a long time; it just didn't work very well
> > before.  And it's not batching the commits better; it's reducing the
> > lock contention around realizing that the batched commit has happened.
> 
> This understanding of group commit is technically accurate, but the
> practical implications of the new code are that lots of people benefit
> from group commit, potentially to rather a large degree, where before
> they had exactly no benefit from group commit. We never officially
> called group commit group commit outside of git commit messages
> before. Therefore, it is sort of like we didn't have group commit
> before but now we do, and it's an implementation that's probably as
> effective as that of any of our competitors. It is for that reason
> that I suggested group commit get a more prominent billing, and that
> it actually be officially referred to as group commit. I'm glad that
> the release notes now actually refer to group commit.
> 
> Now, I realise that as one of the authors of the feature I am open to
> accusations of lacking objectivity - clearly it isn't really my place
> to try and influence the feature's placement, and this is the last I
> will say on the matter unless someone else brings it up again. I just
> think that pedantically characterising this as an improvement to our
> existing group commit implementation within a document that will be
> read like a press release is a bad decision, especially since our
> competitors never had a group commit implementation that was far
> inferior to their current implementation. The assumption will be that
> it's a small improvement that's hardly worth noticing at all.

Thanks for the summary. I know we talk about group commit, but I wasn't
aware that it had not been exposed to our general users.  I agree we
need to reword the item as you suggested.  So this group commit happens
even if users don't change these?

        #commit_delay = 0           # range 0-100000, in microseconds
        #commit_siblings = 5            # range 1-1000

So the new release item wording will be:

        Add group commit capability for sessions that commit at the same
        time

This is the git commit message:

    Make group commit more effective.

    When a backend needs to flush the WAL, and someone else is already flushing
    the WAL, wait until it releases the WALInsertLock and check if we still need
    to do the flush or if the other backend already did the work for us, before
    acquiring WALInsertLock. This helps group commit, because when the WAL flush
    finishes, all the backends that were waiting for it can be woken up in one
    go, and the can all concurrently observe that they're done, rather than
    waking them up one by one in a cascading fashion.

    This is based on a new LWLock function, LWLockWaitUntilFree(), which has
    peculiar semantics. If the lock is immediately free, it grabs the lock and
    returns true. If it's not free, it waits until it is released, but then
    returns false without grabbing the lock. This is used in XLogFlush(), so
    that when the lock is acquired, the backend flushes the WAL, but if it's
    not, the backend first checks the current flush location before retrying.

    Original patch and benchmarking by Peter Geoghegan and Simon Riggs, although
    this patch as committed ended up being very different from that.

    (Heikki Linnakangas)

Is that commit message inaccurate?

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to