Re: [HACKERS] [PATCH] Assert that the correct locks are held when calling PageGetLSN()

On Tue, Jan 09, 2018 at 05:19:00PM -0300, Alvaro Herrera wrote:
> Alvaro Herrera wrote:
> So ... gistdoinsert can sometimes hold an exclusive lock, so we could do
> this instead:
>
> diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
> index 0e499598a4..2ea19d2683 100644
> --- a/src/backend/access/gist/gist.c
> +++ b/src/backend/access/gist/gist.c
> @@ -566,7 +566,8 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate)
> }
>
> stack->page = (Page) BufferGetPage(stack->buffer);
> - stack->lsn = PageGetLSN(stack->page);
> + stack->lsn = xlocked ?
> + PageGetLSN(stack->page) : BufferGetLSNAtomic(stack->buffer);
> Assert(!RelationNeedsWAL(state.r) ||
> !XLogRecPtrIsInvalid(stack->lsn));

Indeed. That's better. Thanks.

> I marked the CF entry as "committed", BTW. I assume you're going to
> ship an updated version of 0002 to the next commitfest. If you have a
> new version during this commitfest, feel free to turn this entry back to
> "needs review".

That's fine for me. The rest can always be revisited later once the
issues raised are addressed.
--
Michael