From 02e644ae9e5575acc6ae09168e4002287a6e2d6e Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 21 Aug 2017 15:10:34 -0700 Subject: [PATCH] Move to BufferGetLSNAtomic where spinlock is needed Certain callers of PageGetLSN weren't correctly holding the buffer spinlock; it is needed whenever the buffer content lock is not held in exclusive mode. For heapam.c, also ensure that we don't access the LSN after releasing the lock. Signed-off-by: Asim R P --- src/backend/access/gist/gist.c | 4 ++-- src/backend/access/gist/gistget.c | 4 ++-- src/backend/access/heap/heapam.c | 11 ++++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 02f973fa9d..4198a1e1dc 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -562,7 +562,7 @@ gistfindleaf(GISTInsertState *state, GISTSTATE *giststate) state->stack->page = (Page) BufferGetPage(state->stack->buffer); opaque = GistPageGetOpaque(state->stack->page); - state->stack->lsn = PageGetLSN(state->stack->page); + state->stack->lsn = BufferGetLSNAtomic(state->stack->buffer); Assert(state->r->rd_istemp || !XLogRecPtrIsInvalid(state->stack->lsn)); if (state->stack->blkno != GIST_ROOT_BLKNO && @@ -699,7 +699,7 @@ gistFindPath(Relation r, BlockNumber child) break; } - top->lsn = PageGetLSN(page); + top->lsn = BufferGetLSNAtomic(buffer); if (top->parent && XLByteLT(top->parent->lsn, GistPageGetOpaque(page)->nsn) && GistPageGetOpaque(page)->rightlink != InvalidBlockNumber /* sanity check */ ) diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c index 4649dd3eba..b3a4df0cc7 100644 --- a/src/backend/access/gist/gistget.c +++ b/src/backend/access/gist/gistget.c @@ -43,7 +43,7 @@ killtuple(Relation r, GISTScanOpaque so, ItemPointer iptr) gistcheckpage(r, so->curbuf); p = (Page) BufferGetPage(so->curbuf); - if (XLByteEQ(so->stack->lsn, PageGetLSN(p))) + if (XLByteEQ(so->stack->lsn, BufferGetLSNAtomic(so->curbuf))) { /* page unchanged, so all is simple */ offset = ItemPointerGetOffsetNumber(iptr); @@ -240,7 +240,7 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, opaque = GistPageGetOpaque(p); /* remember lsn to identify page changed for tuple's killing */ - so->stack->lsn = PageGetLSN(p); + so->stack->lsn = BufferGetLSNAtomic(so->curbuf); /* check page split, occured from last visit or visit to parent */ if (!XLogRecPtrIsInvalid(so->stack->parentlsn) && diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index f0f2c80dce..836cda968c 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -5241,11 +5241,6 @@ heap_xlog_delete(XLogRecPtr lsn, XLogRecord *record) if (XLByteLE(lsn, PageGetLSN(page))) /* changes are applied */ { - UnlockReleaseBuffer(buffer); - - MIRROREDLOCK_BUFMGR_UNLOCK; - // -------- MirroredLock ---------- - if (Debug_print_qd_mirroring) { elog(LOG, "delete already appplied: lsn (%X,%X), page (%X,%X)", @@ -5254,6 +5249,12 @@ heap_xlog_delete(XLogRecPtr lsn, XLogRecord *record) PageGetLSN(page).xlogid, PageGetLSN(page).xrecoff); } + + UnlockReleaseBuffer(buffer); + + MIRROREDLOCK_BUFMGR_UNLOCK; + // -------- MirroredLock ---------- + return; } -- GitLab