Send only one FORGET_RELATION_FSYNC request when dropping a relation.

We were sending one per fork, but a little bit of refactoring allows us
to send just one request with forknum == InvalidForkNumber.  This not only
reduces pressure on the shared-memory request queue, but saves repeated
traversals of the checkpointer's hash table.

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index f074cedca598d3afe8436c66fe097ca001441014..6b3c3ee0e75a8752c5c98f771107027507365f94 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -166,6 +166,8 @@ typedef enum                                        /* behavior for mdopen & _mdfd_getseg */
 } ExtensionBehavior;
 
 /* local routines */
+static void mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum,
+                                                bool isRedo);
 static MdfdVec *mdopen(SMgrRelation reln, ForkNumber forknum,
           ExtensionBehavior behavior);
 static void register_dirty_segment(SMgrRelation reln, ForkNumber forknum,
@@ -308,6 +310,9 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
  * Note that we're passed a RelFileNodeBackend --- by the time this is called,
  * there won't be an SMgrRelation hashtable entry anymore.
  *
+ * forkNum can be a fork number to delete a specific fork, or InvalidForkNumber
+ * to delete all forks.
+ *
  * For regular relations, we don't unlink the first segment file of the rel,
  * but just truncate it to zero length, and record a request to unlink it after
  * the next checkpoint.  Additional segments can be unlinked immediately,
@@ -349,17 +354,32 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
 void
 mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
 {
-       char       *path;
-       int                     ret;
-
        /*
         * We have to clean out any pending fsync requests for the doomed
         * relation, else the next mdsync() will fail.  There can't be any such
-        * requests for a temp relation, though.
+        * requests for a temp relation, though.  We can send just one request
+        * even when deleting multiple forks, since the fsync queuing code accepts
+        * the "InvalidForkNumber = all forks" convention.
         */
        if (!RelFileNodeBackendIsTemp(rnode))
                ForgetRelationFsyncRequests(rnode.node, forkNum);
 
+       /* Now do the per-fork work */
+       if (forkNum == InvalidForkNumber)
+       {
+               for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
+                       mdunlinkfork(rnode, forkNum, isRedo);
+       }
+       else
+               mdunlinkfork(rnode, forkNum, isRedo);
+}
+
+static void
+mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
+{
+       char       *path;
+       int                     ret;
+
        path = relpath(rnode, forkNum);
 
        /*
@@ -1340,7 +1360,8 @@ register_unlink(RelFileNodeBackend rnode)
  * The range of possible segment numbers is way less than the range of
  * BlockNumber, so we can reserve high values of segno for special purposes.
  * We define three:
- * - FORGET_RELATION_FSYNC means to cancel pending fsyncs for a relation
+ * - FORGET_RELATION_FSYNC means to cancel pending fsyncs for a relation,
+ *   either for one fork, or all forks if forknum is InvalidForkNumber
  * - FORGET_DATABASE_FSYNC means to cancel pending fsyncs for a whole database
  * - UNLINK_RELATION_REQUEST is a request to delete the file after the next
  *      checkpoint.
@@ -1356,7 +1377,7 @@ RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
 
        if (segno == FORGET_RELATION_FSYNC)
        {
-               /* Remove any pending requests for the entire relation */
+               /* Remove any pending requests for the relation (one or all forks) */
                HASH_SEQ_STATUS hstat;
                PendingOperationEntry *entry;
 
@@ -1364,7 +1385,8 @@ RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
                while ((entry = (PendingOperationEntry *) hash_seq_search(&hstat)) != NULL)
                {
                        if (RelFileNodeEquals(entry->tag.rnode, rnode) &&
-                               entry->tag.forknum == forknum)
+                               (entry->tag.forknum == forknum ||
+                                forknum == InvalidForkNumber))
                        {
                                /* Okay, cancel this entry */
                                entry->canceled = true;
@@ -1466,6 +1488,9 @@ RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
 
 /*
  * ForgetRelationFsyncRequests -- forget any fsyncs for a relation fork
+ *
+ * forknum == InvalidForkNumber means all forks, although this code doesn't
+ * actually know that, since it's just forwarding the request elsewhere.
  */
 void
 ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum)

diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 407942ace490e079c3d2766ce69fe5688e291e04..0cec1477f3faf6551ad141b5107da1ea70b8b54b 100644 (file)
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -381,8 +381,7 @@ smgrdounlink(SMgrRelation reln, bool isRedo)
         * ERROR, because we've already decided to commit or abort the current
         * xact.
         */
-       for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
-               (*(smgrsw[which].smgr_unlink)) (rnode, forknum, isRedo);
+       (*(smgrsw[which].smgr_unlink)) (rnode, InvalidForkNumber, isRedo);
 }
 
 /*