Re: WalSndWakeup() and synchronous_commit=off

On 6 June 2012 20:11, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On Tuesday, May 29, 2012 08:42:43 PM Andres Freund wrote:
>> Hi,
>>
>> On Monday, May 28, 2012 07:11:53 PM Tom Lane wrote:
>> > Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> > > Does anybody have a better idea than to either call WalSndWakeup() at
>> > > essentially the wrong places or calling it inside a critical section?
>> > >
>> > > Tom, what danger do you see from calling it in a critical section?
>> >
>> > My concern was basically that it might throw an error.  Looking at the
>> > current implementation of SetLatch, it seems that's not possible, but
>> > I wonder whether we want to lock ourselves into that assumption.
>>
>> The assumption is already made at several other places I think.
>> XLogSetAsyncXactLSN does a SetLatch and is called from critical sections;
>> several signal handlers call it without any attention to the context.
>>
>> Requiring it to be called outside would make its usage considerably less
>> convenient and I don't really see what could change that would require to
>> throw non-panic errors.
>>
>> > Still, if the alternatives are worse, maybe that's the best answer.
>> > If we do that, though, let's add comments to WalSndWakeup and SetLatch
>> > mentioning that they mustn't throw error.
>>
>> Patch attached.
> I would like to invite some more review (+commit...) here ;). Imo this is an
> annoying bug which should be fixed before next point release or beta/rc comes
> out...

Moved the wakeup to a logical place outside a critical section.

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