On 2013-08-27 12:17:55 -0500, Merlin Moncure wrote:
> On Tue, Aug 27, 2013 at 10:55 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> > On 2013-08-27 09:57:38 -0500, Merlin Moncure wrote:
> >> + bool
> >> + RecoveryMightBeInProgress(void)
> >> + {
> >> + /*
> >> + * We check shared state each time only until we leave recovery mode. We
> >> + * can't re-enter recovery, so there's no need to keep checking after the
> >> + * shared variable has once been seen false.
> >> + */
> >> + if (!LocalRecoveryInProgress)
> >> + return false;
> >> + else
> >> + {
> >> + /* use volatile pointer to prevent code rearrangement */
> >> + volatile XLogCtlData *xlogctl = XLogCtl;
> >> +
> >> + /* Intentionally query xlogctl without spinlocking! */
> >> + LocalRecoveryInProgress = xlogctl->SharedRecoveryInProgress;
> >> +
> >> + return LocalRecoveryInProgress;
> >> + }
> >> + }
> >
> > I don't think it's acceptable to *set* LocalRecoveryInProgress
> > here. That should only be done in the normal routine.
>
> quite right -- that was a major error -- you could bypass the
> initialization call to the xlog with some bad luck.
I've seen this in profiles since, so I'd appreciate pushing this
forward.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
