Improve the performance of relation deletes during recovery.

When multiple relations are deleted at the same transaction,
the files of those relations are deleted by one call to smgrdounlinkall(),
which leads to scan whole shared_buffers only one time. OTOH,
previously, during recovery, smgrdounlink() (not smgrdounlinkall()) was
called for each file to delete, which led to scan shared_buffers
multiple times. Obviously this could cause to increase the WAL replay
time very much especially when shared_buffers was huge.

To alleviate this situation, this commit changes the recovery so that
it also calls smgrdounlinkall() only one time to delete multiple
relation files.

This is just fix for oversight of commit 279628a0a7, not new feature.
So, per discussion on pgsql-hackers, we concluded to backpatch this
to all supported versions.

Author: Fujii Masao
Reviewed-by: Michael Paquier, Andres Freund, Thomas Munro, Kyotaro Horiguchi, Takayuki Tsunakawa
Discussion: https://postgr.es/m/CAHGQGwHVQkdfDqtvGVkty+19cQakAydXn1etGND3X0PHbZ3+6w@mail.gmail.com

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 94fd721517b04845af0687af5c8a3eb31583c56b..3c732d8f8014a6c5f6d31933499adbd2711650bd 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1361,7 +1361,6 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
        RelFileNode *delrels;
        int                     ndelrels;
        SharedInvalidationMessage *invalmsgs;
-       int                     i;
 
        /*
         * Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -1451,13 +1450,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
                delrels = abortrels;
                ndelrels = hdr->nabortrels;
        }
-       for (i = 0; i < ndelrels; i++)
-       {
-               SMgrRelation srel = smgropen(delrels[i], InvalidBackendId);
 
-               smgrdounlink(srel, false);
-               smgrclose(srel);
-       }
+       /* Make sure files supposed to be dropped are dropped */
+       DropRelationFiles(delrels, ndelrels, false);
 
        /*
         * Handle cache invalidation messages.

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 81d8b7d0d9caa684774b1a32ade48c0837b6dc07..8e6b6305cdf1ef77ca41a58219f3abe56c2cf30c 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -4642,7 +4642,6 @@ xact_redo_commit_internal(TransactionId xid, XLogRecPtr lsn,
                                                  uint32 xinfo)
 {
        TransactionId max_xid;
-       int                     i;
 
        max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
 
@@ -4737,16 +4736,8 @@ xact_redo_commit_internal(TransactionId xid, XLogRecPtr lsn,
                 */
                XLogFlush(lsn);
 
-               for (i = 0; i < nrels; i++)
-               {
-                       SMgrRelation srel = smgropen(xnodes[i], InvalidBackendId);
-                       ForkNumber      fork;
-
-                       for (fork = 0; fork <= MAX_FORKNUM; fork++)
-                               XLogDropRelation(xnodes[i], fork);
-                       smgrdounlink(srel, true);
-                       smgrclose(srel);
-               }
+               /* Make sure files supposed to be dropped are dropped */
+               DropRelationFiles(xnodes, nrels, true);
        }
 
        /*
@@ -4818,7 +4809,6 @@ xact_redo_abort(xl_xact_abort *xlrec, TransactionId xid)
 {
        TransactionId *sub_xids;
        TransactionId max_xid;
-       int                     i;
 
        sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
        max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
@@ -4877,16 +4867,7 @@ xact_redo_abort(xl_xact_abort *xlrec, TransactionId xid)
        }
 
        /* Make sure files supposed to be dropped are dropped */
-       for (i = 0; i < xlrec->nrels; i++)
-       {
-               SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
-               ForkNumber      fork;
-
-               for (fork = 0; fork <= MAX_FORKNUM; fork++)
-                       XLogDropRelation(xlrec->xnodes[i], fork);
-               smgrdounlink(srel, true);
-               smgrclose(srel);
-       }
+       DropRelationFiles(xlrec->xnodes, xlrec->nrels, true);
 }
 
 void

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index b94211676189f760e2194c1866d864e375345e15..fa91484cafdb735f462f5812eccbf159be2d19ed 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -26,6 +26,7 @@
 #include <sys/file.h>
 
 #include "miscadmin.h"
+#include "access/xlogutils.h"
 #include "access/xlog.h"
 #include "catalog/catalog.h"
 #include "common/relpath.h"
@@ -1626,6 +1627,43 @@ ForgetDatabaseFsyncRequests(Oid dbid)
        }
 }
 
+/*
+ * DropRelationFiles -- drop files of all given relations
+ */
+void
+DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo)
+{
+       SMgrRelation *srels;
+       int                     i;
+
+       srels = palloc(sizeof(SMgrRelation) * ndelrels);
+       for (i = 0; i < ndelrels; i++)
+       {
+               SMgrRelation srel = smgropen(delrels[i], InvalidBackendId);
+
+               if (isRedo)
+               {
+                       ForkNumber      fork;
+
+                       for (fork = 0; fork <= MAX_FORKNUM; fork++)
+                               XLogDropRelation(delrels[i], fork);
+               }
+               srels[i] = srel;
+       }
+
+       smgrdounlinkall(srels, ndelrels, isRedo);
+
+       /*
+        * Call smgrclose() in reverse order as when smgropen() is called.
+        * This trick enables remove_from_unowned_list() in smgrclose()
+        * to search the SMgrRelation from the unowned list,
+        * with O(1) performance.
+        */
+       for (i = ndelrels - 1; i >= 0; i--)
+               smgrclose(srels[i]);
+       pfree(srels);
+}
+
 
 /*
  *     _fdvec_alloc() -- Make a MdfdVec object.

diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index d57c30d915b7a7079e83f810193bb6959b576f78..301ff8043b273c16aea4afa9e660560c7d6d5698 100644 (file)
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -135,6 +135,7 @@ extern void RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum,
                                         BlockNumber segno);
 extern void ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum);
 extern void ForgetDatabaseFsyncRequests(Oid dbid);
+extern void DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo);
 
 /* smgrtype.c */
 extern Datum smgrout(PG_FUNCTION_ARGS);