public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
Subject: [PATCH 3/6] Minor cleanup
Date: Mon, 11 Mar 2019 15:15:43 +0200
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 | 9 +--
src/backend/access/gist/gistxlog.c | 88 +++++++++++++++-------------
src/include/access/gist_private.h | 6 +-
src/include/access/gistxlog.h | 10 ++--
5 files changed, 62 insertions(+), 58 deletions(-)
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 6f87b4b5044..e260c4df4e7 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 fc606ac823c..eb90b2077d3 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;
}
@@ -112,7 +109,7 @@ gistvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
* 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
@@ -213,7 +210,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;
@@ -305,7 +302,7 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
LockBuffer(leafBuffer, GIST_EXCLUSIVE);
gistcheckpage(rel, leafBuffer);
leafPage = (Page) BufferGetPage(leafBuffer);
- if (!GistPageIsLeaf(leafPage) /* not a leaf anymore */
+ if (!GistPageIsLeaf(leafPage) /* not a leaf anymore */
|| PageGetMaxOffsetNumber(leafPage) != InvalidOffsetNumber /* Page is not empry */
|| (GistFollowRight(leafPage) || GistPageGetNSN(page) < GistPageGetNSN(leafPage)) /* No follow-right */
)
diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c
index 3213ea98ea3..7110c70451e 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 943163ccce7..c77d0b4dd81 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 127cff5cb78..939a1ea7559 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
--------------AF9416BC788B4CF5E4BF929D
Content-Type: text/x-patch;
name="0004-Move-the-page-deletion-logic-to-separate-function.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0004-Move-the-page-deletion-logic-to-separate-function.patch"
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 3/6] Minor cleanup
In-Reply-To: <no-message-id-1883628@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox