Re: WalSndWakeup() and synchronous_commit=off

On Thursday, June 07, 2012 08:41:23 PM Simon Riggs wrote:
> 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.
Hm. I don't really like the way you implemented that. While it reduces the
likelihood quite a bit it will still miss wakeups if an XLogInsert pushes out
the data because of missing space or if any place does an XLogFlush(lsn).
The knowledge is really only available in XLogWrite...

Andres