Hi,
On 2018-07-15 18:48:43 -0400, Tom Lane wrote:
> So basically, WAL replay hits an error while holding a buffer pin, and
> nothing is done to release the buffer pin, but AtProcExit_Buffers thinks
> something should have been done.
I think there's a few other cases where we hit this. I've seen something
similar from inside checkpointer / BufferSync(). I'd be surprised if
bgwriter couldn't be triggered into the same.
> What seems like a better answer for now is to adjust AtProcExit_Buffers
> so that it doesn't cause an assertion failure in this case. I think we
> can define "this case" as being "failure exit from the startup process":
> if that happens, the postmaster is going to throw up its hands and force
> a panic shutdown anyway, so the failure to free a buffer pin shouldn't
> have any serious consequences.
> The attached one-liner patch does that, and is enough to get through
> Michael's test case without an assertion. This is just proof of concept
> though --- my inclination, if we go this route, is to make a slightly
> longer patch that would fix CheckForBufferLeaks to still print the WARNING
> messages if any, but not die with an assertion afterwards.
>
> Another question is whether this is really a sufficient band-aid. It
> looks to me like AtProcExit_Buffers will be called in any auxiliary
> process type, not only the startup process.
We could just replace the Assert() with a PANIC?
> Do, or should we, force a panic restart for nonzero-exit-code failures
> of all aux process types? If not, what are we going to do to clean up
> similar failures in other aux process types?
I'm pretty sure that we do *not* force a panic on all nonzero-exit-code
cases for other subprocesses.
> BTW, this assertion has been there since fe548629; before that, there
> was code that would release any leaked buffer pins, relying on the
> PrivateRefCount data for that. So another idea is to restore some
> version of that capability, although I think we really really don't
> wanna scan the PrivateRefCount array if we can avoid it.
I don't think scanning PrivateRefCount would be particularly problematic
- in almost all cases it's going to be tiny. These day it's not NBuffers
sized anymore, so I can't forsee any performance problems?
I think we could invent something like like enter/leave "transactional
mode" wherein we throw a PANIC when a buffer leaked. Processes that
don't enter it - like startup, checkpointer, etc - would just WARN?
Greetings,
Andres Freund
