]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix longstanding problems in VACUUM caused by untimely interruptions
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Tue, 10 Nov 2009 18:00:44 +0000 (18:00 +0000)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Tue, 10 Nov 2009 18:00:44 +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 e3ec8772ce2291ad655819223936591a59dad749..b83299172dddc6e90335899437dd9b5616219a25 100644 (file)
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364.2.2 2008/09/11 14:01:35 alvherre Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364.2.3 2009/11/10 18:00:44 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -210,10 +210,10 @@ static List *get_rel_oids(List *relids, const RangeVar *vacrel,
 static void vac_truncate_clog(TransactionId frozenXID);
 static void vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
                                           bool for_wraparound);
-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,
@@ -976,6 +976,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
        Oid                     toast_relid;
        Oid                     save_userid;
        bool            save_secdefcxt;
+       bool            heldoff;
 
        /* Begin a transaction for vacuuming this relation */
        StartTransactionCommand();
@@ -1121,9 +1122,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
         * 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);
+               heldoff = lazy_vacuum_rel(onerel, vacstmt, vac_strategy);
 
        /* Restore userid */
        SetUserIdAndContext(save_userid, save_secdefcxt);
@@ -1136,6 +1137,10 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
         */
        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
@@ -1169,8 +1174,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind,
  *
  *             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
@@ -1181,6 +1189,7 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
        int                     nindexes,
                                i;
        VRelStats  *vacrelstats;
+       bool            heldoff = false;
 
        vacuum_set_xid_limits(vacstmt->freeze_min_age, onerel->rd_rel->relisshared,
                                                  &OldestXmin, &FreezeLimit);
@@ -1231,7 +1240,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);
        }
@@ -1256,6 +1265,8 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
        /* report results to the stats collector, too */
        pgstat_report_vacuum(RelationGetRelid(onerel), onerel->rd_rel->relisshared,
                                                 vacstmt->analyze, vacrelstats->rel_tuples);
+
+       return heldoff;
 }
 
 
@@ -1805,8 +1816,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)
@@ -1831,6 +1845,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);
 
@@ -2635,8 +2650,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();
        }
@@ -2832,6 +2851,8 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
                pfree(vacrelstats->vtlinks);
 
        ExecContext_Finish(&ec);
+
+       return heldoff;
 }
 
 /*
index d9224dc28e5a751f22b1ac9c9fca22ecc965550b..574a14ad814672c400f8dcac1f2f317d38198e92 100644 (file)
@@ -38,7 +38,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.103.2.2 2009/01/06 14:55:44 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.103.2.3 2009/11/10 18:00:44 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -147,8 +147,11 @@ static int vac_cmp_page_spaces(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)
 {
@@ -158,6 +161,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        BlockNumber possibly_freeable;
        PGRUsage        ru0;
        TimestampTz starttime = 0;
+       bool            heldoff = false;
 
        pg_rusage_init(&ru0);
 
@@ -198,12 +202,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);
+       }
 
        /* Update shared free space map with final free space info */
        lazy_update_fsm(onerel, vacrelstats);
@@ -248,6 +262,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
                                                vacrelstats->tuples_deleted, vacrelstats->rel_tuples,
                                                        pg_rusage_show(&ru0))));
        }
+
+       return heldoff;
 }
 
 
index 4beb1f8c785d7d9fca8081fb26c41002977698df..f263c81a00888ed5ce85fd8a550ee7928c804fc5 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/commands/vacuum.h,v 1.75.2.1 2008/03/14 17:26:01 alvherre Exp $
+ * $PostgreSQL: pgsql/src/include/commands/vacuum.h,v 1.75.2.2 2009/11/10 18:00:44 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -131,7 +131,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);
 
 /* in commands/analyze.c */