Fix heap_page_prune's problem with failing to send cache invalidation
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 13 Mar 2008 18:00:32 +0000 (18:00 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 13 Mar 2008 18:00:32 +0000 (18:00 +0000)
messages if the calling transaction aborts later on.  Collapsing out line
pointer redirects is a done deal as soon as we complete the page update,
so syscache *must* be notified even if the VACUUM FULL as a whole doesn't
complete.  To fix, add some functionality to inval.c to allow the pending
inval messages to be sent immediately while heap_page_prune is still
running.  The implementation is a bit chintzy: it will only work in the
context of VACUUM FULL.  But that's all we need now, and it can always be
extended later if needed.  Per my trouble report of a week ago.

src/backend/access/heap/pruneheap.c
src/backend/utils/cache/inval.c
src/include/utils/inval.h

index 09c55ee76155f266b5aa06ef9475d53071941d3e..3db5d0b82e6426e2c427df67587b84f39dea815d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/access/heap/pruneheap.c,v 1.7 2008/03/08 21:57:59 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/access/heap/pruneheap.c,v 1.8 2008/03/13 18:00:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -158,7 +158,14 @@ heap_page_prune(Relation relation, Buffer buffer, TransactionId OldestXmin,
     * much logic as possible out of the critical section, and also ensures
     * that WAL replay will work the same as the normal case.
     *
-    * First, initialize the new pd_prune_xid value to zero (indicating no
+    * First, inform inval.c that upcoming CacheInvalidateHeapTuple calls
+    * are nontransactional.
+    */
+   if (redirect_move)
+       BeginNonTransactionalInvalidation();
+
+   /*
+    * Initialize the new pd_prune_xid value to zero (indicating no
     * prunable tuples).  If we find any tuples which may soon become
     * prunable, we will save the lowest relevant XID in new_prune_xid.
     * Also initialize the rest of our working state.
@@ -191,6 +198,18 @@ heap_page_prune(Relation relation, Buffer buffer, TransactionId OldestXmin,
                                     redirect_move);
    }
 
+   /*
+    * Send invalidation messages for any tuples we are about to move.
+    * It is safe to do this now, even though we could theoretically still
+    * fail before making the actual page update, because a useless cache
+    * invalidation doesn't hurt anything.  Also, no one else can reload the
+    * tuples while we have exclusive buffer lock, so it's not too early to
+    * send the invals.  This avoids sending the invals while inside the
+    * critical section, which is a good thing for robustness.
+    */
+   if (redirect_move)
+       EndNonTransactionalInvalidation();
+
    /* Any error while applying the changes is critical */
    START_CRIT_SECTION();
 
@@ -587,16 +606,10 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
    /*
     * If we are going to implement a redirect by moving tuples, we have
     * to issue a cache invalidation against the redirection target tuple,
-    * because its CTID will be effectively changed by the move.  It is
-    * safe to do this now, even though we might fail before reaching the
-    * actual page update, because a useless cache invalidation doesn't
-    * hurt anything.  Furthermore we really really don't want to issue
-    * the inval inside the critical section, since it has a nonzero
-    * chance of causing an error (eg, due to running out of memory).
-    *
-    * XXX this is not really right because CacheInvalidateHeapTuple
-    * expects transactional semantics, and our move isn't transactional.
-    * FIXME!!
+    * because its CTID will be effectively changed by the move.  Note that
+    * CacheInvalidateHeapTuple only queues the request, it doesn't send it;
+    * if we fail before reaching EndNonTransactionalInvalidation, nothing
+    * happens and no harm is done.
     */
    if (OffsetNumberIsValid(redirect_target))
    {
index 033ae3a2d623a1c5641cb8c173036488b7b77307..ccb002f0f5d82b45c908f7f4617a65ba7b64f19d 100644 (file)
@@ -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.83 2008/01/01 19:45:53 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/cache/inval.c,v 1.84 2008/03/13 18:00:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -964,6 +964,99 @@ CommandEndInvalidationMessages(void)
                               &transInvalInfo->CurrentCmdInvalidMsgs);
 }
 
+
+/*
+ * BeginNonTransactionalInvalidation
+ *     Prepare for invalidation messages for nontransactional updates.
+ *
+ * A nontransactional invalidation is one that must be sent whether or not
+ * the current transaction eventually commits.  We arrange for all invals
+ * queued between this call and EndNonTransactionalInvalidation() to be sent
+ * immediately when the latter is called.
+ *
+ * Currently, this is only used by heap_page_prune(), and only when it is
+ * invoked during VACUUM FULL's first pass over a table.  We expect therefore
+ * that we are not inside a subtransaction and there are no already-pending
+ * invalidations.  This could be relaxed by setting up a new nesting level of
+ * invalidation data, but for now there's no need.  Note that heap_page_prune
+ * knows that this function does not change any state, and therefore there's
+ * no need to worry about cleaning up if there's an elog(ERROR) before
+ * reaching EndNonTransactionalInvalidation (the invals will just be thrown
+ * away if that happens).
+ */
+void
+BeginNonTransactionalInvalidation(void)
+{
+   /* Must be at top of stack */
+   Assert(transInvalInfo != NULL && transInvalInfo->parent == NULL);
+
+   /* Must not have any previously-queued activity */
+   Assert(transInvalInfo->PriorCmdInvalidMsgs.cclist == NULL);
+   Assert(transInvalInfo->PriorCmdInvalidMsgs.rclist == NULL);
+   Assert(transInvalInfo->CurrentCmdInvalidMsgs.cclist == NULL);
+   Assert(transInvalInfo->CurrentCmdInvalidMsgs.rclist == NULL);
+   Assert(transInvalInfo->RelcacheInitFileInval == false);
+}
+
+/*
+ * EndNonTransactionalInvalidation
+ *     Process queued-up invalidation messages for nontransactional updates.
+ *
+ * We expect to find messages in CurrentCmdInvalidMsgs only (else there
+ * was a CommandCounterIncrement within the "nontransactional" update).
+ * We must process them locally and send them out to the shared invalidation
+ * message queue.
+ *
+ * We must also reset the lists to empty and explicitly free memory (we can't
+ * rely on end-of-transaction cleanup for that).
+ */
+void
+EndNonTransactionalInvalidation(void)
+{
+   InvalidationChunk *chunk;
+   InvalidationChunk *next;
+
+   /* Must be at top of stack */
+   Assert(transInvalInfo != NULL && transInvalInfo->parent == NULL);
+
+   /* Must not have any prior-command messages */
+   Assert(transInvalInfo->PriorCmdInvalidMsgs.cclist == NULL);
+   Assert(transInvalInfo->PriorCmdInvalidMsgs.rclist == NULL);
+
+   /*
+    * At present, this function is only used for CTID-changing updates;
+    * since the relcache init file doesn't store any tuple CTIDs, we
+    * don't have to invalidate it.  That might not be true forever
+    * though, in which case we'd need code similar to AtEOXact_Inval.
+    */
+
+   /* Send out the invals */
+   ProcessInvalidationMessages(&transInvalInfo->CurrentCmdInvalidMsgs,
+                               LocalExecuteInvalidationMessage);
+   ProcessInvalidationMessages(&transInvalInfo->CurrentCmdInvalidMsgs,
+                               SendSharedInvalidMessage);
+
+   /* Clean up and release memory */
+   for (chunk = transInvalInfo->CurrentCmdInvalidMsgs.cclist;
+        chunk != NULL;
+        chunk = next)
+   {
+       next = chunk->next;
+       pfree(chunk);
+   }
+   for (chunk = transInvalInfo->CurrentCmdInvalidMsgs.rclist;
+        chunk != NULL;
+        chunk = next)
+   {
+       next = chunk->next;
+       pfree(chunk);
+   }
+   transInvalInfo->CurrentCmdInvalidMsgs.cclist = NULL;
+   transInvalInfo->CurrentCmdInvalidMsgs.rclist = NULL;
+   transInvalInfo->RelcacheInitFileInval = false;
+}
+
+
 /*
  * CacheInvalidateHeapTuple
  *     Register the given tuple for invalidation at end of command
index df0e5f397b35ecc3f36d0f59a60f94bc0cec2fbb..80c8c2ab3312e11600182694f57bf0269144bb90 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/inval.h,v 1.41 2008/01/01 19:45:59 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/utils/inval.h,v 1.42 2008/03/13 18:00:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,6 +37,10 @@ extern void PostPrepare_Inval(void);
 
 extern void CommandEndInvalidationMessages(void);
 
+extern void BeginNonTransactionalInvalidation(void);
+
+extern void EndNonTransactionalInvalidation(void);
+
 extern void CacheInvalidateHeapTuple(Relation relation, HeapTuple tuple);
 
 extern void CacheInvalidateRelcache(Relation relation);