Re: [PERFORM] Cpu usage 100% on slave. s_lock problem.

On Fri, Sep 27, 2013 at 7:56 AM, Merlin Moncure <mmoncure@gmail.com> wrote:
> On Thu, Sep 26, 2013 at 10:14 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
>> On Thu, Sep 26, 2013 at 6:08 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>>> 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.
>>
>> roger that -- will push ahead when I get into the office...
>
> attached is new version fixing some comment typos.

Attached is simplified patch that replaces the spinlock with a read
barrier based on a suggestion made by Andres offlist.  The approach
has different performance characteristics -- a barrier call is being
issued instead of a non-blocking read.   I don't have a performance
test case in hand to prove that's better so I'm going with Andre's
approach because it's simpler.  Aside: can this truly the only caller
of pg_read_barrier()?

Also, moving to -hackers from -performance.

merlin