Re: [HACKERS] [PATCH] Assert that the correct locks are held whencalling 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