Re: Uh, I change my mind about commit_delay + commit_siblings (sort of)

On 27 June 2012 23:01, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> 1. Are there any call sites from which this should be disabled, either
> because the optimization won't help, or because the caller is holding
> a lock that we don't want them sitting on for a moment longer than
> necessary?  I think the worst case is that we're doing something like
> splitting the root page of a btree, and now nobody can walk the btree
> until the flush finishes, and here we are doing an "unnecessary"
> sleep.  I am not sure where I can construct a workload where this is a
> real as opposed to a theoretical problem, though.  So maybe we should
> just not worry about it.  It suffices for this to be an improvement
> over the status quo; it doesn't have to be perfect.  Thoughts?

Let's put this in now, without too much fiddling. There is already a
GUC to disable it, so measurements can be made to see if this causes
any problems.

If there are problems, we fix them. We can't second guess everything.

> 2. Should we rename the GUCs, since this patch will cause them to
> control WAL flush in general, as opposed to commit specifically?
> Peter Geoghegan and Simon were arguing that we should retitle it to
> group_commit_delay rather than just commit_delay, but that doesn't
> seem to be much of an improvement in describing what the new behavior
> will actually be, and I am doubtful that it is worth creating a naming
> incompatibility with previous releases for a cosmetic change.  I
> suggested wal_flush_delay, but there's no consensus on that.
> Opinions?

Again, leave the naming of that for later. The idea of a rename came
from yourself, IIRC.

> The XLByteLE test four lines from the bottom should happen before we
> consider whether to sleep, because it's possible that we'll discover
> that our flush request has already been satisfied and exit without
> doing anything, in which case the sleep is a waste.  The attached
> version adjusts the logic accordingly.

I thought there already was a test like that earlier in the flush.

I agree there needs to be one.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services