From: Heikki Linnakangas Date: Mon, 11 Mar 2019 16:58:10 +0200 Subject: [PATCH 6/6] Misc cleanup. --- src/backend/access/gist/gistvacuum.c | 32 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index bf0b9d54f69..54a0adaf71f 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -419,7 +419,7 @@ gistvacuum_recycle_pages(GistBulkDeleteResult *stats) Buffer buffer; Page page; OffsetNumber off, - maxoff; + maxoff; IndexTuple idxtuple; ItemId iid; OffsetNumber todelete[MaxOffsetNumber]; @@ -436,6 +436,8 @@ gistvacuum_recycle_pages(GistBulkDeleteResult *stats) page = (Page) BufferGetPage(buffer); if (PageIsNew(page) || GistPageIsDeleted(page) || GistPageIsLeaf(page)) { + /* HEIKKI: This page was an internal page earlier, but now it's something else. + * Shouldn't happen... */ UnlockReleaseBuffer(buffer); continue; } @@ -446,12 +448,13 @@ gistvacuum_recycle_pages(GistBulkDeleteResult *stats) off <= maxoff && ntodelete < maxoff-1; off = OffsetNumberNext(off)) { + BlockNumber leafBlockNo; Buffer leafBuffer; Page leafPage; - BlockNumber leafBlockNo; iid = PageGetItemId(page, off); idxtuple = (IndexTuple) PageGetItem(page, iid); + /* if this page was not empty in previous scan - we do not consider it */ leafBlockNo = ItemPointerGetBlockNumber(&(idxtuple->t_tid)); if (!blockset_get(leafBlockNo, stats->emptyLeafPagesMap)) @@ -468,6 +471,7 @@ gistvacuum_recycle_pages(GistBulkDeleteResult *stats) leafPage = (Page) BufferGetPage(leafBuffer); if (!GistPageIsLeaf(leafPage)) { + /* it's not a leaf anymore? shouldn't happen.. */ UnlockReleaseBuffer(leafBuffer); continue; } @@ -483,27 +487,29 @@ gistvacuum_recycle_pages(GistBulkDeleteResult *stats) UnlockReleaseBuffer(leafBuffer); } - /* - * We will have to relock internal page in case of deletes: - * we cannot lock child while holding parent lock without risk - * of a deadlock - */ - LockBuffer(buffer, GIST_UNLOCK); - - if (ntodelete) + if (ntodelete > 0) { TransactionId txid; int i; + /* + * We will have to relock internal page in case of deletes: + * we cannot lock child while holding parent lock without risk + * of a deadlock + */ + LockBuffer(buffer, GIST_UNLOCK); + for (i = 0; i < ntodelete; i++) { Buffer leafBuffer = buftodelete[i]; Page leafPage; + + /* FIXME: Aren't we still holding the lock from the loop above? */ LockBuffer(leafBuffer, GIST_EXCLUSIVE); gistcheckpage(rel, leafBuffer); leafPage = (Page) BufferGetPage(leafBuffer); if (!GistPageIsLeaf(leafPage) /* not a leaf anymore */ - || PageGetMaxOffsetNumber(leafPage) != InvalidOffsetNumber /* Page is not empry */ + || PageGetMaxOffsetNumber(leafPage) != InvalidOffsetNumber /* Page is not empty */ || (GistFollowRight(leafPage) || GistPageGetNSN(page) < GistPageGetNSN(leafPage)) /* No follow-right */ ) { @@ -587,10 +593,8 @@ gistvacuum_recycle_pages(GistBulkDeleteResult *stats) UnlockReleaseBuffer(buftodelete[i]); } END_CRIT_SECTION(); - - LockBuffer(buffer, GIST_UNLOCK); } - ReleaseBuffer(buffer); + UnlockReleaseBuffer(buffer); } } -- 2.20.1 --------------AF9416BC788B4CF5E4BF929D--