]> granicus.if.org Git - postgresql/commitdiff
Close un-owned SMgrRelations at transaction end.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 17 Oct 2012 16:38:28 +0000 (12:38 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 17 Oct 2012 16:38:28 +0000 (12:38 -0400)
If an SMgrRelation is not "owned" by a relcache entry, don't allow it to
live past transaction end.  This design allows the same SMgrRelation to be
used for blind writes of multiple blocks during a transaction, but ensures
that we don't hold onto such an SMgrRelation indefinitely.  Because an
SMgrRelation typically corresponds to open file descriptors at the fd.c
level, leaving it open when there's no corresponding relcache entry can
mean that we prevent the kernel from reclaiming deleted disk space.
(While CacheInvalidateSmgr messages usually fix that, there are cases
where they're not issued, such as DROP DATABASE.  We might want to add
some more sinval messaging for that, but I'd be inclined to keep this
type of logic anyway, since allowing VFDs to accumulate indefinitely
for blind-written relations doesn't seem like a good idea.)

This code replaces a previous attempt towards the same goal that proved
to be unreliable.  Back-patch to 9.1 where the previous patch was added.

src/backend/access/transam/xact.c
src/backend/postmaster/bgwriter.c
src/backend/postmaster/checkpointer.c
src/backend/postmaster/walwriter.c
src/backend/storage/smgr/smgr.c
src/include/storage/smgr.h

index 8f00186dd741b565c2769da496b28b40fbddc881..5f85fe2011622d15932bab92bb3d6e25ec979249 100644 (file)
@@ -1969,7 +1969,7 @@ CommitTransaction(void)
        AtEOXact_SPI(true);
        AtEOXact_on_commit_actions(true);
        AtEOXact_Namespace(true);
-       /* smgrcommit already done */
+       AtEOXact_SMgr();
        AtEOXact_Files();
        AtEOXact_ComboCid();
        AtEOXact_HashTables(true);
@@ -2222,7 +2222,7 @@ PrepareTransaction(void)
        AtEOXact_SPI(true);
        AtEOXact_on_commit_actions(true);
        AtEOXact_Namespace(true);
-       /* smgrcommit already done */
+       AtEOXact_SMgr();
        AtEOXact_Files();
        AtEOXact_ComboCid();
        AtEOXact_HashTables(true);
@@ -2368,6 +2368,7 @@ AbortTransaction(void)
                AtEOXact_SPI(false);
                AtEOXact_on_commit_actions(false);
                AtEOXact_Namespace(false);
+               AtEOXact_SMgr();
                AtEOXact_Files();
                AtEOXact_ComboCid();
                AtEOXact_HashTables(false);
index f98f138e9a6b2ec0273c700b4944da2c57563af0..1cc1ac3d41362f626af6b90e383225a38e800dc4 100644 (file)
@@ -182,6 +182,7 @@ BackgroundWriterMain(void)
                                                         false, true);
                /* we needn't bother with the other ResourceOwnerRelease phases */
                AtEOXact_Buffers(false);
+               AtEOXact_SMgr();
                AtEOXact_Files();
                AtEOXact_HashTables(false);
 
index ec5690bb69c7d0fee2ec4109354b7591e0267757..5f94250af2793612d086207c0922d68618e17cee 100644 (file)
@@ -290,6 +290,7 @@ CheckpointerMain(void)
                                                         false, true);
                /* we needn't bother with the other ResourceOwnerRelease phases */
                AtEOXact_Buffers(false);
+               AtEOXact_SMgr();
                AtEOXact_Files();
                AtEOXact_HashTables(false);
 
index 8a7949558102f7c695595d6f48ea561228c7d4ab..5582c3c295af3b31ca715ae72e8083e076007da3 100644 (file)
@@ -187,6 +187,7 @@ WalWriterMain(void)
                                                         false, true);
                /* we needn't bother with the other ResourceOwnerRelease phases */
                AtEOXact_Buffers(false);
+               AtEOXact_SMgr();
                AtEOXact_Files();
                AtEOXact_HashTables(false);
 
index 6319d1e8589138be32e498e77cc4ce484211c1a1..5dff8b3702ee64c355a050aa76242b99b6eea151 100644 (file)
@@ -76,11 +76,15 @@ static const int NSmgr = lengthof(smgrsw);
 
 /*
  * Each backend has a hashtable that stores all extant SMgrRelation objects.
+ * In addition, "unowned" SMgrRelation objects are chained together in a list.
  */
 static HTAB *SMgrRelationHash = NULL;
 
+static SMgrRelation first_unowned_reln = NULL;
+
 /* local function prototypes */
 static void smgrshutdown(int code, Datum arg);
+static void remove_from_unowned_list(SMgrRelation reln);
 
 
 /*
@@ -124,7 +128,7 @@ smgrshutdown(int code, Datum arg)
 /*
  *     smgropen() -- Return an SMgrRelation object, creating it if need be.
  *
- *             This does not attempt to actually open the object.
+ *             This does not attempt to actually open the underlying file.
  */
 SMgrRelation
 smgropen(RelFileNode rnode, BackendId backend)
@@ -144,6 +148,7 @@ smgropen(RelFileNode rnode, BackendId backend)
                ctl.hash = tag_hash;
                SMgrRelationHash = hash_create("smgr relation table", 400,
                                                                           &ctl, HASH_ELEM | HASH_FUNCTION);
+               first_unowned_reln = NULL;
        }
 
        /* Look up or create an entry */
@@ -168,6 +173,10 @@ smgropen(RelFileNode rnode, BackendId backend)
                /* mark it not open */
                for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
                        reln->md_fd[forknum] = NULL;
+
+               /* place it at head of unowned list (to make smgrsetowner cheap) */
+               reln->next_unowned_reln = first_unowned_reln;
+               first_unowned_reln = reln;
        }
 
        return reln;
@@ -182,20 +191,60 @@ smgropen(RelFileNode rnode, BackendId backend)
 void
 smgrsetowner(SMgrRelation *owner, SMgrRelation reln)
 {
+       /* We don't currently support "disowning" an SMgrRelation here */
+       Assert(owner != NULL);
+
        /*
         * First, unhook any old owner.  (Normally there shouldn't be any, but it
         * seems possible that this can happen during swap_relation_files()
         * depending on the order of processing.  It's ok to close the old
         * relcache entry early in that case.)
+        *
+        * If there isn't an old owner, then the reln should be in the unowned
+        * list, and we need to remove it.
         */
        if (reln->smgr_owner)
                *(reln->smgr_owner) = NULL;
+       else
+               remove_from_unowned_list(reln);
 
        /* Now establish the ownership relationship. */
        reln->smgr_owner = owner;
        *owner = reln;
 }
 
+/*
+ * remove_from_unowned_list -- unlink an SMgrRelation from the unowned list
+ *
+ * If the reln is not present in the list, nothing happens.  Typically this
+ * would be caller error, but there seems no reason to throw an error.
+ *
+ * In the worst case this could be rather slow; but in all the cases that seem
+ * likely to be performance-critical, the reln being sought will actually be
+ * first in the list.  Furthermore, the number of unowned relns touched in any
+ * one transaction shouldn't be all that high typically.  So it doesn't seem
+ * worth expending the additional space and management logic needed for a
+ * doubly-linked list.
+ */
+static void
+remove_from_unowned_list(SMgrRelation reln)
+{
+       SMgrRelation *link;
+       SMgrRelation cur;
+
+       for (link = &first_unowned_reln, cur = *link;
+                cur != NULL;
+                link = &cur->next_unowned_reln, cur = *link)
+       {
+               if (cur == reln)
+               {
+                       *link = cur->next_unowned_reln;
+                       cur->next_unowned_reln = NULL;
+                       break;
+               }
+       }
+}
+
 /*
  *     smgrexists() -- Does the underlying file for a fork exist?
  */
@@ -219,6 +268,9 @@ smgrclose(SMgrRelation reln)
 
        owner = reln->smgr_owner;
 
+       if (!owner)
+               remove_from_unowned_list(reln);
+
        if (hash_search(SMgrRelationHash,
                                        (void *) &(reln->smgr_rnode),
                                        HASH_REMOVE, NULL) == NULL)
@@ -600,3 +652,29 @@ smgrpostckpt(void)
                        (*(smgrsw[i].smgr_post_ckpt)) ();
        }
 }
+
+/*
+ * AtEOXact_SMgr
+ *
+ * This routine is called during transaction commit or abort (it doesn't
+ * particularly care which).  All transient SMgrRelation objects are closed.
+ *
+ * We do this as a compromise between wanting transient SMgrRelations to
+ * live awhile (to amortize the costs of blind writes of multiple blocks)
+ * and needing them to not live forever (since we're probably holding open
+ * a kernel file descriptor for the underlying file, and we need to ensure
+ * that gets closed reasonably soon if the file gets deleted).
+ */
+void
+AtEOXact_SMgr(void)
+{
+       /*
+        * Zap all unowned SMgrRelations.  We rely on smgrclose() to remove each
+        * one from the list.
+        */
+       while (first_unowned_reln != NULL)
+       {
+               Assert(first_unowned_reln->smgr_owner == NULL);
+               smgrclose(first_unowned_reln);
+       }
+}
index 92b9198d5428c1f51617f595d062da1bc8af5106..9eccf7671ed99be4c068e3df50a75c21c1ce9e51 100644 (file)
@@ -33,6 +33,9 @@
  * without having to make the smgr explicitly aware of relcache.  There
  * can't be more than one "owner" pointer per SMgrRelation, but that's
  * all we need.
+ *
+ * SMgrRelations that do not have an "owner" are considered to be transient,
+ * and are deleted at end of transaction.
  */
 typedef struct SMgrRelationData
 {
@@ -63,6 +66,9 @@ typedef struct SMgrRelationData
 
        /* for md.c; NULL for forks that are not open */
        struct _MdfdVec *md_fd[MAX_FORKNUM + 1];
+
+       /* if unowned, list link in list of all unowned SMgrRelations */
+       struct SMgrRelationData *next_unowned_reln;
 } SMgrRelationData;
 
 typedef SMgrRelationData *SMgrRelation;
@@ -95,6 +101,7 @@ extern void smgrimmedsync(SMgrRelation reln, ForkNumber forknum);
 extern void smgrpreckpt(void);
 extern void smgrsync(void);
 extern void smgrpostckpt(void);
+extern void AtEOXact_SMgr(void);
 
 
 /* internals: move me elsewhere -- ay 7/94 */