Separate the functions of relcache entry flush and smgr cache entry flush
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 10 Jan 2005 21:57:19 +0000 (21:57 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 10 Jan 2005 21:57:19 +0000 (21:57 +0000)
so that we can get the size of a shared inval message back down to what it
was in 7.4 (and simplify the logic too).  Phase 2 of fixing the
'SMgrRelation hashtable corrupted' problem.

src/backend/utils/cache/inval.c
src/include/storage/sinval.h

index 8933030a9dd2977ffd6a3a81eb6f6efadd07bbb9..2195458982918955e47278682351fd52be74acf1 100644 (file)
  *
  * Also, whenever we see an operation on a pg_class or pg_attribute tuple,
  * we register a relcache flush operation for the relation described by that
- * tuple.
- *
- * We keep the relcache flush requests in lists separate from the catcache
- * tuple flush requests.  This allows us to issue all the pending catcache
- * flushes before we issue relcache flushes, which saves us from loading
- * a catcache tuple during relcache load only to flush it again right away.
- * Also, we avoid queuing multiple relcache flush requests for the same
- * relation, since a relcache flush is relatively expensive to do.
+ * tuple.  pg_class updates trigger an smgr flush operation as well.
+ *
+ * We keep the relcache and smgr flush requests in lists separate from the
+ * catcache tuple flush requests.  This allows us to issue all the pending
+ * catcache flushes before we issue relcache flushes, which saves us from
+ * loading a catcache tuple during relcache load only to flush it again
+ * right away.  Also, we avoid queuing multiple relcache flush requests for
+ * the same relation, since a relcache flush is relatively expensive to do.
  * (XXX is it worth testing likewise for duplicate catcache flush entries?
  * Probably not.)
  *
@@ -80,7 +80,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/cache/inval.c,v 1.69 2005/01/10 20:02:23 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/cache/inval.c,v 1.70 2005/01/10 21:57:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -115,7 +115,7 @@ typedef struct InvalidationChunk
 typedef struct InvalidationListHeader
 {
    InvalidationChunk *cclist;  /* list of chunks holding catcache msgs */
-   InvalidationChunk *rclist;  /* list of chunks holding relcache msgs */
+   InvalidationChunk *rclist;  /* list of chunks holding relcache/smgr msgs */
 } InvalidationListHeader;
 
 /*----------------
@@ -164,7 +164,7 @@ static TransInvalidationInfo *transInvalInfo = NULL;
 
 static struct CACHECALLBACK
 {
-   int16       id;             /* cache number or SHAREDINVALRELCACHE_ID */
+   int16       id;             /* cache number or message type id */
    CacheCallbackFunction function;
    Datum       arg;
 }  cache_callback_list[MAX_CACHE_CALLBACKS];
@@ -273,7 +273,7 @@ AppendInvalidationMessageList(InvalidationChunk **destHdr,
  *             Invalidation set support functions
  *
  * These routines understand about the division of a logical invalidation
- * list into separate physical lists for catcache and relcache entries.
+ * list into separate physical lists for catcache and relcache/smgr entries.
  * ----------------------------------------------------------------
  */
 
@@ -299,22 +299,42 @@ AddCatcacheInvalidationMessage(InvalidationListHeader *hdr,
  */
 static void
 AddRelcacheInvalidationMessage(InvalidationListHeader *hdr,
-                              Oid dbId, Oid relId, RelFileNode physId)
+                              Oid dbId, Oid relId)
 {
    SharedInvalidationMessage msg;
 
    /* Don't add a duplicate item */
    /* We assume dbId need not be checked because it will never change */
-   /* relfilenode fields must be checked to support reassignment */
    ProcessMessageList(hdr->rclist,
-                      if (msg->rc.relId == relId &&
-                     RelFileNodeEquals(msg->rc.physId, physId)) return);
+                      if (msg->rc.id == SHAREDINVALRELCACHE_ID &&
+                          msg->rc.relId == relId)
+                          return);
 
    /* OK, add the item */
    msg.rc.id = SHAREDINVALRELCACHE_ID;
    msg.rc.dbId = dbId;
    msg.rc.relId = relId;
-   msg.rc.physId = physId;
+   AddInvalidationMessage(&hdr->rclist, &msg);
+}
+
+/*
+ * Add an smgr inval entry
+ */
+static void
+AddSmgrInvalidationMessage(InvalidationListHeader *hdr,
+                          RelFileNode rnode)
+{
+   SharedInvalidationMessage msg;
+
+   /* Don't add a duplicate item */
+   ProcessMessageList(hdr->rclist,
+                      if (msg->sm.id == SHAREDINVALSMGR_ID &&
+                          RelFileNodeEquals(msg->sm.rnode, rnode))
+                          return);
+
+   /* OK, add the item */
+   msg.sm.id = SHAREDINVALSMGR_ID;
+   msg.sm.rnode = rnode;
    AddInvalidationMessage(&hdr->rclist, &msg);
 }
 
@@ -370,10 +390,10 @@ RegisterCatcacheInvalidation(int cacheId,
  * As above, but register a relcache invalidation event.
  */
 static void
-RegisterRelcacheInvalidation(Oid dbId, Oid relId, RelFileNode physId)
+RegisterRelcacheInvalidation(Oid dbId, Oid relId)
 {
    AddRelcacheInvalidationMessage(&transInvalInfo->CurrentCmdInvalidMsgs,
-                                  dbId, relId, physId);
+                                  dbId, relId);
 
    /*
     * If the relation being invalidated is one of those cached in the
@@ -383,10 +403,22 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId, RelFileNode physId)
        transInvalInfo->RelcacheInitFileInval = true;
 }
 
+/*
+ * RegisterSmgrInvalidation
+ *
+ * As above, but register an smgr invalidation event.
+ */
+static void
+RegisterSmgrInvalidation(RelFileNode rnode)
+{
+   AddSmgrInvalidationMessage(&transInvalInfo->CurrentCmdInvalidMsgs,
+                              rnode);
+}
+
 /*
  * LocalExecuteInvalidationMessage
  *
- * Process a single invalidation message (which could be either type).
+ * Process a single invalidation message (which could be of any type).
  * Only the local caches are flushed; this does not transmit the message
  * to other backends.
  */
@@ -426,17 +458,14 @@ LocalExecuteInvalidationMessage(SharedInvalidationMessage *msg)
                    (*ccitem->function) (ccitem->arg, msg->rc.relId);
            }
        }
+   }
+   else if (msg->id == SHAREDINVALSMGR_ID)
+   {
        /*
-        * If the message includes a valid relfilenode, we must ensure
-        * the smgr cache entry gets zapped.  This might not have happened
-        * above since the relcache entry might not have existed or might
-        * have been associated with a different relfilenode.
-        *
-        * XXX there is no real good reason for rnode inval to be in the
-        * same message at all.  FIXME in 8.1.
+        * We could have smgr entries for relations of other databases,
+        * so no short-circuit test is possible here.
         */
-       if (OidIsValid(msg->rc.physId.relNode))
-           smgrclosenode(msg->rc.physId);
+       smgrclosenode(msg->sm.rnode);
    }
    else
        elog(FATAL, "unrecognized SI message id: %d", msg->id);
@@ -475,16 +504,11 @@ InvalidateSystemCaches(void)
  *     of catalog/relation cache entries; if so, register inval events.
  */
 static void
-PrepareForTupleInvalidation(Relation relation, HeapTuple tuple,
-                           void (*CacheIdRegisterFunc) (int, uint32,
-                                                      ItemPointer, Oid),
-                           void (*RelationIdRegisterFunc) (Oid, Oid,
-                                                           RelFileNode))
+PrepareForTupleInvalidation(Relation relation, HeapTuple tuple)
 {
    Oid         tupleRelId;
    Oid         databaseId;
    Oid         relationId;
-   RelFileNode rnode;
 
    /* Do nothing during bootstrap */
    if (IsBootstrapProcessingMode())
@@ -510,7 +534,7 @@ PrepareForTupleInvalidation(Relation relation, HeapTuple tuple,
     * First let the catcache do its thing
     */
    PrepareToInvalidateCacheTuple(relation, tuple,
-                                 CacheIdRegisterFunc);
+                                 RegisterCatcacheInvalidation);
 
    /*
     * Now, is this tuple one of the primary definers of a relcache entry?
@@ -520,27 +544,36 @@ PrepareForTupleInvalidation(Relation relation, HeapTuple tuple,
    if (tupleRelId == RelOid_pg_class)
    {
        Form_pg_class classtup = (Form_pg_class) GETSTRUCT(tuple);
+       RelFileNode rnode;
 
        relationId = HeapTupleGetOid(tuple);
        if (classtup->relisshared)
            databaseId = InvalidOid;
        else
            databaseId = MyDatabaseId;
-       if (classtup->reltablespace)
-           rnode.spcNode = classtup->reltablespace;
-       else
-           rnode.spcNode = MyDatabaseTableSpace;
-       rnode.dbNode = databaseId;
-       rnode.relNode = classtup->relfilenode;
 
        /*
+        * We need to send out an smgr inval as well as a relcache inval.
+        * This is needed because other backends might possibly possess
+        * smgr cache but not relcache entries for the target relation.
+        *
         * Note: during a pg_class row update that assigns a new
         * relfilenode or reltablespace value, we will be called on both
         * the old and new tuples, and thus will broadcast invalidation
         * messages showing both the old and new RelFileNode values.  This
         * ensures that other backends will close smgr references to the
         * old file.
+        *
+        * XXX possible future cleanup: it might be better to trigger smgr
+        * flushes explicitly, rather than indirectly from pg_class updates.
         */
+       if (classtup->reltablespace)
+           rnode.spcNode = classtup->reltablespace;
+       else
+           rnode.spcNode = MyDatabaseTableSpace;
+       rnode.dbNode = databaseId;
+       rnode.relNode = classtup->relfilenode;
+       RegisterSmgrInvalidation(rnode);
    }
    else if (tupleRelId == RelOid_pg_attribute)
    {
@@ -558,10 +591,6 @@ PrepareForTupleInvalidation(Relation relation, HeapTuple tuple,
         * though.
         */
        databaseId = MyDatabaseId;
-       /* We assume no smgr cache flush is needed, either */
-       rnode.spcNode = InvalidOid;
-       rnode.dbNode = InvalidOid;
-       rnode.relNode = InvalidOid;
    }
    else
        return;
@@ -569,7 +598,7 @@ PrepareForTupleInvalidation(Relation relation, HeapTuple tuple,
    /*
     * Yes.  We need to register a relcache invalidation event.
     */
-   (*RelationIdRegisterFunc) (databaseId, relationId, rnode);
+   RegisterRelcacheInvalidation(databaseId, relationId);
 }
 
 
@@ -790,9 +819,7 @@ CommandEndInvalidationMessages(void)
 void
 CacheInvalidateHeapTuple(Relation relation, HeapTuple tuple)
 {
-   PrepareForTupleInvalidation(relation, tuple,
-                               RegisterCatcacheInvalidation,
-                               RegisterRelcacheInvalidation);
+   PrepareForTupleInvalidation(relation, tuple);
 }
 
 /*
@@ -803,7 +830,10 @@ CacheInvalidateHeapTuple(Relation relation, HeapTuple tuple)
  * This is used in places that need to force relcache rebuild but aren't
  * changing any of the tuples recognized as contributors to the relcache
  * entry by PrepareForTupleInvalidation.  (An example is dropping an index.)
- * We assume in particular that relfilenode isn't changing.
+ * We assume in particular that relfilenode/reltablespace aren't changing
+ * (so the rd_node value is still good).
+ *
+ * XXX most callers of this probably don't need to force an smgr flush.
  */
 void
 CacheInvalidateRelcache(Relation relation)
@@ -817,7 +847,8 @@ CacheInvalidateRelcache(Relation relation)
    else
        databaseId = MyDatabaseId;
 
-   RegisterRelcacheInvalidation(databaseId, relationId, relation->rd_node);
+   RegisterRelcacheInvalidation(databaseId, relationId);
+   RegisterSmgrInvalidation(relation->rd_node);
 }
 
 /*
@@ -844,7 +875,8 @@ CacheInvalidateRelcacheByTuple(HeapTuple classTuple)
    rnode.dbNode = databaseId;
    rnode.relNode = classtup->relfilenode;
 
-   RegisterRelcacheInvalidation(databaseId, relationId, rnode);
+   RegisterRelcacheInvalidation(databaseId, relationId);
+   RegisterSmgrInvalidation(rnode);
 }
 
 /*
index 0fce7466149c57a471aa87a2400d7e23652fd377..092b722048817bdb2f424f18ceb81ea1f0b7d1d9 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/sinval.h,v 1.39 2004/12/31 22:03:42 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/storage/sinval.h,v 1.40 2005/01/10 21:57:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 
 
 /*
- * We currently support two types of shared-invalidation messages: one that
- * invalidates an entry in a catcache, and one that invalidates a relcache
- * entry.  More types could be added if needed.  The message type is
- * identified by the first "int16" field of the message struct.  Zero or
- * positive means a catcache inval message (and also serves as the catcache
- * ID field).  -1 means a relcache inval message.  Other negative values
- * are available to identify other inval message types.
+ * We currently support three types of shared-invalidation messages: one that
+ * invalidates an entry in a catcache, one that invalidates a relcache entry,
+ * and one that invalidates an smgr cache entry.  More types could be added
+ * if needed.  The message type is identified by the first "int16" field of
+ * the message struct.  Zero or positive means a catcache inval message (and
+ * also serves as the catcache ID field).  -1 means a relcache inval message.
+ * -2 means an smgr inval message.  Other negative values are available to
+ * identify other inval message types.
  *
- * Relcache invalidation messages usually also cause invalidation of entries
- * in the smgr's relation cache.  This means they must carry both logical
- * and physical relation ID info (ie, both dbOID/relOID and RelFileNode).
- * In some cases RelFileNode information is not available so the sender fills
- * those fields with zeroes --- this is okay so long as no smgr cache flush
- * is required.
- *
- * Shared-inval events are initially driven by detecting tuple inserts,
+ * Catcache inval events are initially driven by detecting tuple inserts,
  * updates and deletions in system catalogs (see CacheInvalidateHeapTuple).
  * An update generates two inval events, one for the old tuple and one for
  * the new --- this is needed to get rid of both positive entries for the
@@ -71,20 +65,22 @@ typedef struct
    int16       id;             /* type field --- must be first */
    Oid         dbId;           /* database ID, or 0 if a shared relation */
    Oid         relId;          /* relation ID */
-   RelFileNode physId;         /* physical file ID */
-
-   /*
-    * Note: it is likely that RelFileNode will someday be changed to
-    * include database ID.  In that case the dbId field will be redundant
-    * and should be removed to save space.
-    */
 } SharedInvalRelcacheMsg;
 
+#define SHAREDINVALSMGR_ID     (-2)
+
+typedef struct
+{
+   int16       id;             /* type field --- must be first */
+   RelFileNode rnode;          /* physical file ID */
+} SharedInvalSmgrMsg;
+
 typedef union
 {
    int16       id;             /* type field --- must be first */
    SharedInvalCatcacheMsg cc;
    SharedInvalRelcacheMsg rc;
+   SharedInvalSmgrMsg sm;
 } SharedInvalidationMessage;