From: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
---|---|
To: | Peter Geoghegan <peter(at)2ndquadrant(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, PG Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Uh, I change my mind about commit_delay + commit_siblings (sort of) |
Date: | 2012-05-31 10:19:41 |
Message-ID: | CA+U5nMKSTgUK3qM9qhaRnWUKc6nYf4X0M6R=egj6C2bvDG0HFg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 30 May 2012 17:19, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> On 30 May 2012 15:25, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>> 1. It seems wrong to do it in xact_redo_commit_internal(). It won't
>>> matter if commit_siblings>0 since there won't be any other backends
>>> with transaction IDs anyway, but if commit_siblings==0 then we'll
>>> sleep for no possible benefit.
>>
>> Agreed
I've looked at this more closely now and I can see that the call to
XLogFlush() that is made from xact_redo_commit_internal() doesn't ever
actually flush WAL, so whether we delay or not is completely
irrelevant.
So un-agreed. No change required to patch there.
>>> 2. Doing it in FlushBuffer() seems slightly iffy since we might be
>>> sitting on a buffer lock. But maybe it's a win anyway, or just not
>>> worth worrying about.
>>
>> Agreed.
>
> As I've pointed out, we cannot meaningfully skip the wait for
> auxiliary processes alone (except perhaps by having commit_siblings
> set sufficiently high).
>
>> The remaining cases aren't worth worrying about, apart from
>> SlruPhysicalWritePage() which happens during visibility checks and
>> needs to happen as quickly as possible also.
>
> I'm not so sure. It says in that function:
>
> /*
> * We must determine the largest async-commit LSN for the page. This
> * is a bit tedious, but since this entire function is a slow path
> * anyway, it seems better to do this here than to maintain a per-page
> * LSN variable (which'd need an extra comparison in the
> * transaction-commit path).
> */
>
>> I would say the additional contention from waiting outweighs the
>> benefit of the wait in those 3 places, so skipping the wait is wise.
>
> MinimumActiveBackends() reports the "count backends (other than
> myself) that are in active transactions", so unnecessary calls will
> have to occur when we have active transactions >= CommitSiblings, not
> connections >= CommitSiblings as was previously the case.
>
> What if we were to skip the wait during recovery only, by specially
> setting CommitDelay to 0 in the start-up process? Would that satisfy
> everyone's concerns about unhelpful delays? I'm not sure how this
> might interact with hot standby.
Hmm, that was a good idea, but as of my comments above, that isn't
required or useful.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Alexander Korotkov | 2012-05-31 10:34:51 | Re: GiST buffering build, bug in levelStep calculation |
Previous Message | Heikki Linnakangas | 2012-05-31 09:36:04 | Re: GiST buffering build, bug in levelStep calculation |