Re: Checkpoint start logging is done inside critical section

Hi,

On 2018-10-18 10:21:39 +0530, Amit Kapila wrote:
> On Thu, Oct 18, 2018 at 4:44 AM Ants Aasma <ants(dot)aasma(at)eesti(dot)ee> wrote:
> >
> > The LogCheckpointStart() call inside CreateCheckPoint() is done while
> > inside a critical section. The elog call could trigger errors due to
> > memory allocations or from a logging hook, resulting in a panic.
> >
>
> Yeah, but we use logging hook inside LWLockAcquire as well which is
> also called inside critical section in the nearby code, not sure if we
> can do anything about it.

> > It
> > seems better to postpone the logging until after the critical section
> > is done. It's only a few lwlock acquisitions away and shouldn't make
> > any material difference. Patch to do so is attached.
> >
>
> +1. I think we should backpatch this as well. I can take it forward
> unless people have objections to it.

We do a fair bit of allocations inside a critical section during a
checkpoint (that's why we mark the ctx as being ok with that). I've a
very hard time seing this small allocation be a problem. I definitely
don't think there's a need to backpatch that.

Greetings,

Andres Freund