On 2015-02-13 23:05:16 +0000, Kevin Grittner wrote:
> Andres Freund <andres@2ndquadrant.com> wrote:
>
> > How did you get to that recipe?
>
> I have been working on some patches to allow vacuum to function in
> the face of long-held snapshots. (I'm struggling to get them into
> presentable shape for the upcoming CF.) I was devising the most
> diabolical cases I could to try to break my patched code and
> started seeing this error. I was panicked that I had introduced
> the bug, but on comparing to the master branch I found I was able
> to cause it there, too. So I saw this a couple days before the
> report on list, and had some cases that *sometimes* caused the
> error. I tweaked until it seemed to be pretty reliable, and then
> used that for the bisect.
>
> I still consider you to be the uncontested champion of diabolical
> test cases, but I'm happy to have hit upon one that was useful
> here. ;-)
Hah. Not sure if that's something to be proud of :P
I don't think it's actually 675333 at fault here. I think it's a
long standing bug in LockBufferForCleanup() that can just much easier be
hit with the new interrupt code.
Imagine what happens in LockBufferForCleanup() when ProcWaitForSignal()
returns spuriously - something it's documented to possibly do (and which
got more likely with the new patches). In the normal case UnpinBuffer()
will have unset BM_PIN_COUNT_WAITER - but in a spurious return it'll
still be set and LockBufferForCleanup() will see it still set.
If you just gdb into the VACUUM process with 6647248e370884 checked out,
and do a PGSemaphoreUnlock(&MyProc->sem) you'll hit it as well. I think
we should simply move the buf->flags &= ~BM_PIN_COUNT_WAITER (Inside
LockBuffer) to LockBufferForCleanup, besides the PinCountWaitBuf =
NULL. Afaics, that should do the trick.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
