Fix longstanding problems in VACUUM caused by untimely interruptions
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Tue, 10 Nov 2009 18:00:06 +0000 (18:00 +0000)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Tue, 10 Nov 2009 18:00:06 +0000 (18:00 +0000)
In VACUUM FULL, an interrupt after the initial transaction has been recorded
as committed can cause postmaster to restart with the following error message:
PANIC: cannot abort transaction NNNN, it was already committed
This problem has been reported many times.

In lazy VACUUM, an interrupt after the table has been truncated by
lazy_truncate_heap causes other backends' relcache to still point to the
removed pages; this can cause future INSERT and UPDATE queries to error out
with the following error message:
could not read block XX of relation 1663/NNN/MMMM: read only 0 of 8192 bytes
The window to this race condition is extremely narrow, but it has been seen in
the wild involving a cancelled autovacuum process.

The solution for both problems is to inhibit interrupts in both operations
until after the respective transactions have been committed.  It's not a
complete solution, because the transaction could theoretically be aborted by
some other error, but at least fixes the most common causes of both problems.

src/backend/commands/vacuum.c
src/backend/commands/vacuumlazy.c
src/include/commands/vacuum.h

index e375fb4dae46254f1c37be8f24785804f2a4d3d1..bb1a2077ffa9aed063d50d3ad2966216d64feb73 100644 (file)
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.394 2009/10/26 02:26:29 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.395 2009/11/10 18:00:06 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -217,10 +217,10 @@ static List *get_rel_oids(Oid relid, const RangeVar *vacrel,
 static void vac_truncate_clog(TransactionId frozenXID);
 static void vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast,
                   bool for_wraparound, bool *scanned_all);
-static void full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt);
+static bool full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt);
 static void scan_heap(VRelStats *vacrelstats, Relation onerel,
                  VacPageList vacuum_pages, VacPageList fraged_pages);
-static void repair_frag(VRelStats *vacrelstats, Relation onerel,
+static bool repair_frag(VRelStats *vacrelstats, Relation onerel,
                        VacPageList vacuum_pages, VacPageList fraged_pages,
                        int nindexes, Relation *Irel);
 static void move_chain_tuple(Relation rel,
@@ -1020,6 +1020,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
        Oid                     toast_relid;
        Oid                     save_userid;
        bool            save_secdefcxt;
+       bool            heldoff;
 
        if (scanned_all)
                *scanned_all = false;
@@ -1186,9 +1187,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
         * Do the actual work --- either FULL or "lazy" vacuum
         */
        if (vacstmt->full)
-               full_vacuum_rel(onerel, vacstmt);
+               heldoff = full_vacuum_rel(onerel, vacstmt);
        else
-               lazy_vacuum_rel(onerel, vacstmt, vac_strategy, scanned_all);
+               heldoff = lazy_vacuum_rel(onerel, vacstmt, vac_strategy, scanned_all);
 
        /* Restore userid */
        SetUserIdAndContext(save_userid, save_secdefcxt);
@@ -1202,6 +1203,10 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
        PopActiveSnapshot();
        CommitTransactionCommand();
 
+       /* now we can allow interrupts again, if disabled */
+       if (heldoff)
+               RESUME_INTERRUPTS();
+
        /*
         * If the relation has a secondary toast rel, vacuum that too while we
         * still hold the session lock on the master table.  Note however that
@@ -1235,8 +1240,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
  *
  *             At entry, we have already established a transaction and opened
  *             and locked the relation.
+ *
+ *             The return value indicates whether this function has held off
+ *             interrupts -- caller must RESUME_INTERRUPTS() after commit if true.
  */
-static void
+static bool
 full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
 {
        VacPageListData vacuum_pages;           /* List of pages to vacuum and/or
@@ -1247,6 +1255,7 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
        int                     nindexes,
                                i;
        VRelStats  *vacrelstats;
+       bool            heldoff = false;
 
        vacuum_set_xid_limits(vacstmt->freeze_min_age, vacstmt->freeze_table_age,
                                                  onerel->rd_rel->relisshared,
@@ -1298,7 +1307,7 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
        if (fraged_pages.num_pages > 0)
        {
                /* Try to shrink heap */
-               repair_frag(vacrelstats, onerel, &vacuum_pages, &fraged_pages,
+               heldoff = repair_frag(vacrelstats, onerel, &vacuum_pages, &fraged_pages,
                                        nindexes, Irel);
                vac_close_indexes(nindexes, Irel, NoLock);
        }
@@ -1324,6 +1333,8 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
        /* report results to the stats collector, too */
        pgstat_report_vacuum(RelationGetRelid(onerel), onerel->rd_rel->relisshared,
                                                 true, vacstmt->analyze, vacrelstats->rel_tuples);
+
+       return heldoff;
 }
 
 
@@ -1874,8 +1885,11 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
  *             for them after committing (in hack-manner - without losing locks
  *             and freeing memory!) current transaction. It truncates relation
  *             if some end-blocks are gone away.
+ *
+ *             The return value indicates whether this function has held off
+ *             interrupts -- caller must RESUME_INTERRUPTS() after commit if true.
  */
-static void
+static bool
 repair_frag(VRelStats *vacrelstats, Relation onerel,
                        VacPageList vacuum_pages, VacPageList fraged_pages,
                        int nindexes, Relation *Irel)
@@ -1900,6 +1914,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
        int                     keep_tuples = 0;
        int                     keep_indexed_tuples = 0;
        PGRUsage        ru0;
+       bool            heldoff = false;
 
        pg_rusage_init(&ru0);
 
@@ -2705,8 +2720,12 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                 *
                 * XXX This desperately needs to be revisited.  Any failure after this
                 * point will result in a PANIC "cannot abort transaction nnn, it was
-                * already committed"!
+                * already committed"!  As a precaution, we prevent cancel interrupts
+                * after this point to mitigate this problem; caller is responsible for
+                * re-enabling them after committing the transaction.
                 */
+               HOLD_INTERRUPTS();
+               heldoff = true;
                ForceSyncCommit();
                (void) RecordTransactionCommit();
        }
@@ -2906,6 +2925,8 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                pfree(vacrelstats->vtlinks);
 
        ExecContext_Finish(&ec);
+
+       return heldoff;
 }
 
 /*
index 8fb06e4a68b6fa5f680842188125aa7bb1b8a1df..92fee334ff4030c91461567cb2ca7193814023bf 100644 (file)
@@ -29,7 +29,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.122 2009/08/24 02:18:32 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.123 2009/11/10 18:00:06 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -140,8 +140,11 @@ static int vac_cmp_itemptr(const void *left, const void *right);
  *
  *             At entry, we have already established a transaction and opened
  *             and locked the relation.
+ *
+ *             The return value indicates whether this function has held off
+ *             interrupts -- caller must RESUME_INTERRUPTS() after commit if true.
  */
-void
+bool
 lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
                                BufferAccessStrategy bstrategy, bool *scanned_all)
 {
@@ -153,6 +156,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        TimestampTz starttime = 0;
        bool            scan_all;
        TransactionId freezeTableLimit;
+       bool            heldoff = false;
 
        pg_rusage_init(&ru0);
 
@@ -194,12 +198,22 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
         *
         * Don't even think about it unless we have a shot at releasing a goodly
         * number of pages.  Otherwise, the time taken isn't worth it.
+        *
+        * Note that after we've truncated the heap, it's too late to abort the
+        * transaction; doing so would lose the sinval messages needed to tell
+        * the other backends about the table being shrunk.  We prevent interrupts
+        * in that case; caller is responsible for re-enabling them after
+        * committing the transaction.
         */
        possibly_freeable = vacrelstats->rel_pages - vacrelstats->nonempty_pages;
        if (possibly_freeable > 0 &&
                (possibly_freeable >= REL_TRUNCATE_MINIMUM ||
                 possibly_freeable >= vacrelstats->rel_pages / REL_TRUNCATE_FRACTION))
+       {
+               HOLD_INTERRUPTS();
+               heldoff = true;
                lazy_truncate_heap(onerel, vacrelstats);
+       }
 
        /* Vacuum the Free Space Map */
        FreeSpaceMapVacuum(onerel);
@@ -246,6 +260,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 
        if (scanned_all)
                *scanned_all = vacrelstats->scanned_all;
+
+       return heldoff;
 }
 
 
index b7dff7b0c45c85ddf7754b6028d030d22162312a..bd313f3184715194911458e2c34a548956ec515c 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/vacuum.h,v 1.85 2009/06/11 14:49:11 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/commands/vacuum.h,v 1.86 2009/11/10 18:00:06 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -146,7 +146,7 @@ extern bool vac_is_partial_index(Relation indrel);
 extern void vacuum_delay_point(void);
 
 /* in commands/vacuumlazy.c */
-extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
+extern bool lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
                                BufferAccessStrategy bstrategy, bool *scanned_all);
 
 /* in commands/analyze.c */