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 bda746bf63dfdd93d9027c7d0f0f885fc15e630b..1482fb4c958919ca12f185456fe9a39d48dc2f76 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1390,7 +1390,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
@@ -1482,13 +1481,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 d43819a3e7f96207890bcd29ce126a9c55f56810..df3dc1c168a5a7ead1af2eb1ff3e66956852a762 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -5373,7 +5373,6 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
                                 RepOriginId origin_id)
 {
        TransactionId max_xid;
-       int                     i;
        TimestampTz commit_time;
 
        Assert(TransactionIdIsValid(xid));
@@ -5494,16 +5493,8 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
                 */
                XLogFlush(lsn);
 
-               for (i = 0; i < parsed->nrels; i++)
-               {
-                       SMgrRelation srel = smgropen(parsed->xnodes[i], InvalidBackendId);
-                       ForkNumber      fork;
-
-                       for (fork = 0; fork <= MAX_FORKNUM; fork++)
-                               XLogDropRelation(parsed->xnodes[i], fork);
-                       smgrdounlink(srel, true);
-                       smgrclose(srel);
-               }
+               /* Make sure files supposed to be dropped are dropped */
+               DropRelationFiles(parsed->xnodes, parsed->nrels, true);
        }
 
        /*
@@ -5542,7 +5533,6 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
 static void
 xact_redo_abort(xl_xact_parsed_abort *parsed, TransactionId xid)
 {
-       int                     i;
        TransactionId max_xid;
 
        Assert(TransactionIdIsValid(xid));
@@ -5607,16 +5597,7 @@ xact_redo_abort(xl_xact_parsed_abort *parsed, TransactionId xid)
        }
 
        /* Make sure files supposed to be dropped are dropped */
-       for (i = 0; i < parsed->nrels; i++)
-       {
-               SMgrRelation srel = smgropen(parsed->xnodes[i], InvalidBackendId);
-               ForkNumber      fork;
-
-               for (fork = 0; fork <= MAX_FORKNUM; fork++)
-                       XLogDropRelation(parsed->xnodes[i], fork);
-               smgrdounlink(srel, true);
-               smgrclose(srel);
-       }
+       DropRelationFiles(parsed->xnodes, parsed->nrels, true);
 }
 
 void

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 65e0abe9ec79c5d0884ad55321dad58ee329baee..f487354f671300a6d87bba09b87033147d964e40 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 "pgstat.h"
@@ -1704,6 +1705,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_resize() -- Resize the fork's open segments array

diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 227913458853eacaa791fc448988917ba7ca7052..03eabd3c06bf836da21a97af7899c5f9f0a51876 100644 (file)
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -143,5 +143,6 @@ 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);
 
 #endif                                                 /* SMGR_H */