From b7e482b287f88c2c679f67e7ca46636097252d2a Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 11 Mar 2019 15:15:43 +0200 Subject: [PATCH 3/6] Minor cleanup v22 I renamed gistXLogSetDeleted to gistXLogPageDelete. I think that's better, the WAL record also includes information about removing the downlink from the parent, not just setting the flag on the child. --- src/backend/access/gist/gist.c | 7 +-- src/backend/access/gist/gistvacuum.c | 7 +-- src/backend/access/gist/gistxlog.c | 88 +++++++++++++++------------- src/include/access/gist_private.h | 6 +- src/include/access/gistxlog.h | 10 ++-- 5 files changed, 61 insertions(+), 57 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 6f87b4b504..e260c4df4e 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -844,11 +844,10 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, } /* - * Leaf pages can be left deleted but still referenced - * until it's space is reused. Downlink to this page may be already - * removed from the internal page, but this scan can posess it. + * The page might have been deleted after we scanned the parent + * and saw the downlink. */ - if(GistPageIsDeleted(stack->page)) + if (GistPageIsDeleted(stack->page)) { UnlockReleaseBuffer(stack->buffer); xlocked = false; diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index 85b9d7c219..99dbf1cbb7 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -20,11 +20,9 @@ #include "commands/vacuum.h" #include "lib/blockset.h" #include "miscadmin.h" -#include "nodes/bitmapset.h" #include "storage/indexfsm.h" #include "storage/lmgr.h" - /* Working state needed by gistbulkdelete */ typedef struct { @@ -58,7 +56,6 @@ gistbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, gistvacuumscan(info, stats, callback, callback_state); - return stats; } @@ -163,7 +160,7 @@ gistdeletepage(GistVacState *vstate, * while the index is being expanded, leaving an all-zeros page behind. * * The caller is responsible for initially allocating/zeroing a stats struct. - * + * * Bulk deletion of all index entries pointing to a set of heap tuples and * check invalid tuples left after upgrade. * The set of target tuples is specified via a callback routine that tells @@ -264,7 +261,7 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, stats->num_pages = num_pages; stats->pages_free = vstate.totFreePages; - /* rescan all inner pages to find those that has empty child pages */ + /* rescan all inner pages to find those that have empty child pages */ if (vstate.emptyPages > 0) { BlockNumber x; diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c index 3213ea98ea..7110c70451 100644 --- a/src/backend/access/gist/gistxlog.c +++ b/src/backend/access/gist/gistxlog.c @@ -64,39 +64,6 @@ gistRedoClearFollowRight(XLogReaderState *record, uint8 block_id) UnlockReleaseBuffer(buffer); } -static void -gistRedoPageSetDeleted(XLogReaderState *record) -{ - XLogRecPtr lsn = record->EndRecPtr; - gistxlogPageDelete *xldata = (gistxlogPageDelete *) XLogRecGetData(record); - Buffer buffer; - Page page; - - if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO) - { - page = (Page) BufferGetPage(buffer); - - GistPageSetDeleteXid(page, xldata->deleteXid); - GistPageSetDeleted(page); - - PageSetLSN(page, lsn); - MarkBufferDirty(buffer); - } - if (BufferIsValid(buffer)) - UnlockReleaseBuffer(buffer); - - if (XLogReadBufferForRedo(record, 1, &buffer) == BLK_NEEDS_REDO) - { - page = (Page) BufferGetPage(buffer); - - PageIndexTupleDelete(page, xldata->downlinkOffset); - - PageSetLSN(page, lsn); - MarkBufferDirty(buffer); - } - if (BufferIsValid(buffer)) - UnlockReleaseBuffer(buffer); -} /* * redo any page update (except page split) */ @@ -542,6 +509,43 @@ gistRedoCreateIndex(XLogReaderState *record) UnlockReleaseBuffer(buffer); } +/* redo page deletion */ +static void +gistRedoPageDelete(XLogReaderState *record) +{ + XLogRecPtr lsn = record->EndRecPtr; + gistxlogPageDelete *xldata = (gistxlogPageDelete *) XLogRecGetData(record); + Buffer buffer; + Page page; + + /* FIXME: Are we locking the pages in correct order, for hot standby? */ + + if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO) + { + page = (Page) BufferGetPage(buffer); + + GistPageSetDeleteXid(page, xldata->deleteXid); + GistPageSetDeleted(page); + + PageSetLSN(page, lsn); + MarkBufferDirty(buffer); + } + if (BufferIsValid(buffer)) + UnlockReleaseBuffer(buffer); + + if (XLogReadBufferForRedo(record, 1, &buffer) == BLK_NEEDS_REDO) + { + page = (Page) BufferGetPage(buffer); + + PageIndexTupleDelete(page, xldata->downlinkOffset); + + PageSetLSN(page, lsn); + MarkBufferDirty(buffer); + } + if (BufferIsValid(buffer)) + UnlockReleaseBuffer(buffer); +} + void gist_redo(XLogReaderState *record) { @@ -570,7 +574,7 @@ gist_redo(XLogReaderState *record) gistRedoCreateIndex(record); break; case XLOG_GIST_PAGE_DELETE: - gistRedoPageSetDeleted(record); + gistRedoPageDelete(record); break; default: elog(PANIC, "gist_redo: unknown op code %u", info); @@ -691,25 +695,27 @@ gistXLogSplit(bool page_is_leaf, } /* - * Write XLOG record describing a page delete. This also includes removal of - * downlink from internal page. + * Write XLOG record describing a page deletion. This also includes removal of + * downlink from the parent page. */ XLogRecPtr -gistXLogSetDeleted(RelFileNode node, Buffer buffer, TransactionId xid, - Buffer internalPageBuffer, OffsetNumber internalPageOffset) { +gistXLogPageDelete(Buffer buffer, TransactionId xid, + Buffer parentBuffer, OffsetNumber downlinkOffset) +{ gistxlogPageDelete xlrec; XLogRecPtr recptr; xlrec.deleteXid = xid; - xlrec.downlinkOffset = internalPageOffset; + xlrec.downlinkOffset = downlinkOffset; XLogBeginInsert(); XLogRegisterData((char *) &xlrec, sizeof(gistxlogPageDelete)); XLogRegisterBuffer(0, buffer, REGBUF_STANDARD); - XLogRegisterBuffer(1, internalPageBuffer, REGBUF_STANDARD); - /* new tuples */ + XLogRegisterBuffer(1, parentBuffer, REGBUF_STANDARD); + recptr = XLogInsert(RM_GIST_ID, XLOG_GIST_PAGE_DELETE); + return recptr; } diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index 943163ccce..c77d0b4dd8 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -415,9 +415,9 @@ extern SplitedPageLayout *gistSplit(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *giststate); /* gistxlog.c */ -extern XLogRecPtr gistXLogSetDeleted(RelFileNode node, Buffer buffer, - TransactionId xid, Buffer internalPageBuffer, - OffsetNumber internalPageOffset); +extern XLogRecPtr gistXLogPageDelete(Buffer buffer, + TransactionId xid, Buffer parentBuffer, + OffsetNumber downlinkOffset); extern XLogRecPtr gistXLogUpdate(Buffer buffer, OffsetNumber *todelete, int ntodelete, diff --git a/src/include/access/gistxlog.h b/src/include/access/gistxlog.h index 127cff5cb7..939a1ea755 100644 --- a/src/include/access/gistxlog.h +++ b/src/include/access/gistxlog.h @@ -17,8 +17,6 @@ #include "access/xlogreader.h" #include "lib/stringinfo.h" -/* XLog stuff */ - #define XLOG_GIST_PAGE_UPDATE 0x00 #define XLOG_GIST_DELETE 0x10 /* delete leaf index tuples for a page */ /* #define XLOG_GIST_NEW_ROOT 0x20 */ /* not used anymore */ @@ -78,10 +76,14 @@ typedef struct gistxlogPageSplit */ } gistxlogPageSplit; +/* + * Backup Blk 0: page that was deleted. + * Backup Blk 1: parent page, containing the downlink to the deleted page. + */ typedef struct gistxlogPageDelete { - TransactionId deleteXid; /* last Xid which could see page in scan */ - OffsetNumber downlinkOffset; /* Offset of the downlink referencing this page */ + TransactionId deleteXid; /* last Xid which could see page in scan */ + OffsetNumber downlinkOffset; /* Offset of downlink referencing this page */ } gistxlogPageDelete; extern void gist_redo(XLogReaderState *record); -- 2.20.1