]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.
authorRobert Haas <rhaas@postgresql.org>
Thu, 24 Mar 2022 18:49:08 +0000 (14:49 -0400)
committerRobert Haas <rhaas@postgresql.org>
Thu, 24 Mar 2022 18:49:08 +0000 (14:49 -0400)
If TRUNCATE causes some buffers to be invalidated and thus the
checkpoint does not flush them, TRUNCATE must also ensure that the
corresponding files are truncated on disk. Otherwise, a replay
from the checkpoint might find that the buffers exist but have
the wrong contents, which may cause replay to fail.

Report by Teja Mupparti. Patch by Kyotaro Horiguchi, per a design
suggestion from Heikki Linnakangas, with some changes to the
comments by me. Review of this and a prior patch that approached
the issue differently by Heikki Linnakangas, Andres Freund, Álvaro
Herrera, Masahiko Sawada, and Tom Lane.

Discussion: http://postgr.es/m/BYAPR06MB6373BF50B469CA393C614257ABF00@BYAPR06MB6373.namprd06.prod.outlook.com

src/backend/access/transam/multixact.c
src/backend/access/transam/twophase.c
src/backend/access/transam/xact.c
src/backend/access/transam/xlog.c
src/backend/access/transam/xloginsert.c
src/backend/catalog/storage.c
src/backend/storage/buffer/bufmgr.c
src/backend/storage/ipc/procarray.c
src/backend/storage/lmgr/proc.c
src/include/storage/proc.h
src/include/storage/procarray.h

index ad9e7ff8f059158bb33e6712f3cf6c5db6112588..5612db0e21d17f907a638c86b7f9c1748de7b28d 100644 (file)
@@ -3069,8 +3069,8 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB)
         * crash/basebackup, even though the state of the data directory would
         * require it.
         */
-       Assert(!MyPgXact->delayChkpt);
-       MyPgXact->delayChkpt = true;
+       Assert((MyPgXact->delayChkpt & DELAY_CHKPT_START) == 0);
+       MyPgXact->delayChkpt |= DELAY_CHKPT_START;
 
        /* WAL log truncation */
        WriteMTruncateXlogRec(newOldestMultiDB,
@@ -3096,7 +3096,7 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB)
        /* Then offsets */
        PerformOffsetsTruncation(oldestMulti, newOldestMulti);
 
-       MyPgXact->delayChkpt = false;
+       MyPgXact->delayChkpt &= ~DELAY_CHKPT_START;
 
        END_CRIT_SECTION();
        LWLockRelease(MultiXactTruncationLock);
index 8b402c3a1d73cc731c0a108644d99910edcacf4b..769a5fd71435dae9d2cb941cd7c97959aa150db6 100644 (file)
@@ -476,7 +476,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
        }
        pgxact->xid = xid;
        pgxact->xmin = InvalidTransactionId;
-       pgxact->delayChkpt = false;
+       pgxact->delayChkpt = 0;
        pgxact->vacuumFlags = 0;
        proc->pid = 0;
        proc->databaseId = databaseid;
@@ -1175,7 +1175,8 @@ EndPrepare(GlobalTransaction gxact)
 
        START_CRIT_SECTION();
 
-       MyPgXact->delayChkpt = true;
+       Assert((MyPgXact->delayChkpt & DELAY_CHKPT_START) == 0);
+       MyPgXact->delayChkpt |= DELAY_CHKPT_START;
 
        XLogBeginInsert();
        for (record = records.head; record != NULL; record = record->next)
@@ -1218,7 +1219,7 @@ EndPrepare(GlobalTransaction gxact)
         * checkpoint starting after this will certainly see the gxact as a
         * candidate for fsyncing.
         */
-       MyPgXact->delayChkpt = false;
+       MyPgXact->delayChkpt &= ~DELAY_CHKPT_START;
 
        /*
         * Remember that we have this GlobalTransaction entry locked for us.  If
@@ -2352,7 +2353,8 @@ RecordTransactionCommitPrepared(TransactionId xid,
        START_CRIT_SECTION();
 
        /* See notes in RecordTransactionCommit */
-       MyPgXact->delayChkpt = true;
+       Assert((MyPgXact->delayChkpt & DELAY_CHKPT_START) == 0);
+       MyPgXact->delayChkpt |= DELAY_CHKPT_START;
 
        /*
         * Emit the XLOG commit record. Note that we mark 2PC commits as
@@ -2400,7 +2402,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
        TransactionIdCommitTree(xid, nchildren, children);
 
        /* Checkpoint can proceed now */
-       MyPgXact->delayChkpt = false;
+       MyPgXact->delayChkpt &= ~DELAY_CHKPT_START;
 
        END_CRIT_SECTION();
 
index e32b05d17fb7beab845658af9cfea4c19cf8ef61..5a86b6575e376dc9ce0b7979800886a475ee7581 100644 (file)
@@ -1239,8 +1239,9 @@ RecordTransactionCommit(void)
                 * This makes checkpoint's determination of which xacts are delayChkpt
                 * a bit fuzzy, but it doesn't matter.
                 */
+               Assert((MyPgXact->delayChkpt & DELAY_CHKPT_START) == 0);
                START_CRIT_SECTION();
-               MyPgXact->delayChkpt = true;
+               MyPgXact->delayChkpt |= DELAY_CHKPT_START;
 
                SetCurrentTransactionStopTimestamp();
 
@@ -1341,7 +1342,7 @@ RecordTransactionCommit(void)
         */
        if (markXidCommitted)
        {
-               MyPgXact->delayChkpt = false;
+               MyPgXact->delayChkpt &= ~DELAY_CHKPT_START;
                END_CRIT_SECTION();
        }
 
index b541d0673d2115056168560299fa4dd10d76c1a1..cc0229b934e94718bd27fd04546dd7c684045b74 100644 (file)
@@ -9064,18 +9064,30 @@ CreateCheckPoint(int flags)
         * and we will correctly flush the update below.  So we cannot miss any
         * xacts we need to wait for.
         */
-       vxids = GetVirtualXIDsDelayingChkpt(&nvxids);
+       vxids = GetVirtualXIDsDelayingChkpt(&nvxids, DELAY_CHKPT_START);
        if (nvxids > 0)
        {
                do
                {
                        pg_usleep(10000L);      /* wait for 10 msec */
-               } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids));
+               } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids,
+                                                                                         DELAY_CHKPT_START));
        }
        pfree(vxids);
 
        CheckPointGuts(checkPoint.redo, flags);
 
+       vxids = GetVirtualXIDsDelayingChkpt(&nvxids, DELAY_CHKPT_COMPLETE);
+       if (nvxids > 0)
+       {
+               do
+               {
+                       pg_usleep(10000L);      /* wait for 10 msec */
+               } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids,
+                                                                                         DELAY_CHKPT_COMPLETE));
+       }
+       pfree(vxids);
+
        /*
         * Take a snapshot of running transactions and write this to WAL. This
         * allows us to reconstruct the state of running transactions during
index c033e7bd4cf21b4e95249194857d18bab5664b24..a8c140b06f14eeaf1666ab4eee77efb507cb817f 100644 (file)
@@ -899,7 +899,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std)
        /*
         * Ensure no checkpoint can change our view of RedoRecPtr.
         */
-       Assert(MyPgXact->delayChkpt);
+       Assert((MyPgXact->delayChkpt & DELAY_CHKPT_START) != 0);
 
        /*
         * Update RedoRecPtr so that we can make the right decision
index 5df4382b7e70dae4c86e3682a6cc73f29a40c64f..5d6f456c7085a89e394380f2ae6a9e5db3de8b34 100644 (file)
@@ -27,6 +27,7 @@
 #include "catalog/storage.h"
 #include "catalog/storage_xlog.h"
 #include "storage/freespace.h"
+#include "storage/proc.h"
 #include "storage/smgr.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
@@ -248,6 +249,22 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
        if (vm)
                visibilitymap_truncate(rel, nblocks);
 
+       /*
+        * Make sure that a concurrent checkpoint can't complete while truncation
+        * is in progress.
+        *
+        * The truncation operation might drop buffers that the checkpoint
+        * otherwise would have flushed. If it does, then it's essential that
+        * the files actually get truncated on disk before the checkpoint record
+        * is written. Otherwise, if reply begins from that checkpoint, the
+        * to-be-truncated blocks might still exist on disk but have older
+        * contents than expected, which can cause replay to fail. It's OK for
+        * the blocks to not exist on disk at all, but not for them to have the
+        * wrong contents.
+        */
+       Assert((MyPgXact->delayChkpt & DELAY_CHKPT_COMPLETE) == 0);
+       MyPgXact->delayChkpt |= DELAY_CHKPT_COMPLETE;
+
        /*
         * We WAL-log the truncation before actually truncating, which means
         * trouble if the truncation fails. If we then crash, the WAL replay
@@ -286,8 +303,15 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
                        XLogFlush(lsn);
        }
 
-       /* Do the real work */
+       /*
+        * This will first remove any buffers from the buffer pool that should no
+        * longer exist after truncation is complete, and then truncate the
+        * corresponding files on disk.
+        */
        smgrtruncate(rel->rd_smgr, MAIN_FORKNUM, nblocks);
+
+       /* We've done all the critical work, so checkpoints are OK now. */
+       MyPgXact->delayChkpt &= ~DELAY_CHKPT_COMPLETE;
 }
 
 /*
index 459151519a667f05f1a49f125700c189f66a0e01..027d5067a0ce7cfc7b2b5dab39697bad1a24104f 100644 (file)
@@ -3471,7 +3471,9 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
                         * essential that CreateCheckpoint waits for virtual transactions
                         * rather than full transactionids.
                         */
-                       MyPgXact->delayChkpt = delayChkpt = true;
+                       Assert((MyPgXact->delayChkpt & DELAY_CHKPT_START) == 0);
+                       MyPgXact->delayChkpt |= DELAY_CHKPT_START;
+                       delayChkpt = true;
                        lsn = XLogSaveBufferForHint(buffer, buffer_std);
                }
 
@@ -3504,7 +3506,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
                UnlockBufHdr(bufHdr, buf_state);
 
                if (delayChkpt)
-                       MyPgXact->delayChkpt = false;
+                       MyPgXact->delayChkpt &= ~DELAY_CHKPT_START;
 
                if (dirtied)
                {
index 465ca66857786aabd703faebec965f8e3cc7b47f..d88d9550910457033c3b805686bceb470770ec29 100644 (file)
@@ -433,7 +433,10 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
                pgxact->xmin = InvalidTransactionId;
                /* must be cleared with xid/xmin: */
                pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK;
-               pgxact->delayChkpt = false; /* be sure this is cleared in abort */
+
+               /* be sure this is cleared in abort */
+               pgxact->delayChkpt = 0;
+
                proc->recoveryConflictPending = false;
 
                Assert(pgxact->nxids == 0);
@@ -455,7 +458,10 @@ ProcArrayEndTransactionInternal(PGPROC *proc, PGXACT *pgxact,
        pgxact->xmin = InvalidTransactionId;
        /* must be cleared with xid/xmin: */
        pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK;
-       pgxact->delayChkpt = false; /* be sure this is cleared in abort */
+
+       /* be sure this is cleared in abort */
+       pgxact->delayChkpt = 0;
+
        proc->recoveryConflictPending = false;
 
        /* Clear the subtransaction-XID cache too while holding the lock */
@@ -2267,7 +2273,8 @@ GetOldestSafeDecodingTransactionId(bool catalogOnly)
  * delaying checkpoint because they have critical actions in progress.
  *
  * Constructs an array of VXIDs of transactions that are currently in commit
- * critical sections, as shown by having delayChkpt set in their PGXACT.
+ * critical sections, as shown by having specified delayChkpt bits set in their
+ * PGXACT.
  *
  * Returns a palloc'd array that should be freed by the caller.
  * *nvxids is the number of valid entries.
@@ -2281,13 +2288,15 @@ GetOldestSafeDecodingTransactionId(bool catalogOnly)
  * for clearing of delayChkpt to propagate is unimportant for correctness.
  */
 VirtualTransactionId *
-GetVirtualXIDsDelayingChkpt(int *nvxids)
+GetVirtualXIDsDelayingChkpt(int *nvxids, int type)
 {
        VirtualTransactionId *vxids;
        ProcArrayStruct *arrayP = procArray;
        int                     count = 0;
        int                     index;
 
+       Assert(type != 0);
+
        /* allocate what's certainly enough result space */
        vxids = (VirtualTransactionId *)
                palloc(sizeof(VirtualTransactionId) * arrayP->maxProcs);
@@ -2300,7 +2309,7 @@ GetVirtualXIDsDelayingChkpt(int *nvxids)
                volatile PGPROC *proc = &allProcs[pgprocno];
                volatile PGXACT *pgxact = &allPgXact[pgprocno];
 
-               if (pgxact->delayChkpt)
+               if ((pgxact->delayChkpt & type) != 0)
                {
                        VirtualTransactionId vxid;
 
@@ -2326,12 +2335,14 @@ GetVirtualXIDsDelayingChkpt(int *nvxids)
  * those numbers should be small enough for it not to be a problem.
  */
 bool
-HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids)
+HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids, int type)
 {
        bool            result = false;
        ProcArrayStruct *arrayP = procArray;
        int                     index;
 
+       Assert(type != 0);
+
        LWLockAcquire(ProcArrayLock, LW_SHARED);
 
        for (index = 0; index < arrayP->numProcs; index++)
@@ -2343,7 +2354,8 @@ HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids)
 
                GET_VXID_FROM_PGPROC(vxid, *proc);
 
-               if (pgxact->delayChkpt && VirtualTransactionIdIsValid(vxid))
+               if ((pgxact->delayChkpt & type) != 0 &&
+                       VirtualTransactionIdIsValid(vxid))
                {
                        int                     i;
 
index 69a1e3728926e2230f07ec01620e59e722a6952f..aaecfa67b7fd9995ce2abae5fce5b9fcbae2fea9 100644 (file)
@@ -380,7 +380,7 @@ InitProcess(void)
        MyProc->roleId = InvalidOid;
        MyProc->tempNamespaceId = InvalidOid;
        MyProc->isBackgroundWorker = IsBackgroundWorker;
-       MyPgXact->delayChkpt = false;
+       MyPgXact->delayChkpt = 0;
        MyPgXact->vacuumFlags = 0;
        /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */
        if (IsAutoVacuumWorkerProcess())
@@ -562,7 +562,7 @@ InitAuxiliaryProcess(void)
        MyProc->roleId = InvalidOid;
        MyProc->tempNamespaceId = InvalidOid;
        MyProc->isBackgroundWorker = IsBackgroundWorker;
-       MyPgXact->delayChkpt = false;
+       MyPgXact->delayChkpt = 0;
        MyPgXact->vacuumFlags = 0;
        MyProc->lwWaiting = false;
        MyProc->lwWaitMode = 0;
index 95c9592b211077e7576fe3a8309882d100dc0293..e76ca8a11e87bb2ee657a5bc409f3eefbddd8a18 100644 (file)
@@ -76,6 +76,41 @@ struct XidCache
  */
 #define INVALID_PGPROCNO               PG_INT32_MAX
 
+/*
+ * Flags for PGPROC.delayChkpt
+ *
+ * These flags can be used to delay the start or completion of a checkpoint
+ * for short periods. A flag is in effect if the corresponding bit is set in
+ * the PGPROC of any backend.
+ *
+ * For our purposes here, a checkpoint has three phases: (1) determine the
+ * location to which the redo pointer will be moved, (2) write all the
+ * data durably to disk, and (3) WAL-log the checkpoint.
+ *
+ * Setting DELAY_CHKPT_START prevents the system from moving from phase 1
+ * to phase 2. This is useful when we are performing a WAL-logged modification
+ * of data that will be flushed to disk in phase 2. By setting this flag
+ * before writing WAL and clearing it after we've both written WAL and
+ * performed the corresponding modification, we ensure that if the WAL record
+ * is inserted prior to the new redo point, the corresponding data changes will
+ * also be flushed to disk before the checkpoint can complete. (In the
+ * extremely common case where the data being modified is in shared buffers
+ * and we acquire an exclusive content lock on the relevant buffers before
+ * writing WAL, this mechanism is not needed, because phase 2 will block
+ * until we release the content lock and then flush the modified data to
+ * disk.)
+ *
+ * Setting DELAY_CHKPT_COMPLETE prevents the system from moving from phase 2
+ * to phase 3. This is useful if we are performing a WAL-logged operation that
+ * might invalidate buffers, such as relation truncation. In this case, we need
+ * to ensure that any buffers which were invalidated and thus not flushed by
+ * the checkpoint are actaully destroyed on disk. Replay can cope with a file
+ * or block that doesn't exist, but not with a block that has the wrong
+ * contents.
+ */
+#define DELAY_CHKPT_START              (1<<0)
+#define DELAY_CHKPT_COMPLETE   (1<<1)
+
 /*
  * Each backend has a PGPROC struct in shared memory.  There is also a list of
  * currently-unused PGPROC structs that will be reallocated to new backends.
@@ -232,8 +267,7 @@ typedef struct PGXACT
 
        uint8           vacuumFlags;    /* vacuum-related flags, see above */
        bool            overflowed;
-       bool            delayChkpt;             /* true if this proc delays checkpoint start;
-                                                                * previously called InCommit */
+       int                     delayChkpt;             /* for DELAY_CHKPT_* flags */
 
        uint8           nxids;
 } PGXACT;
index a3a1bf724cc7ecfa138b11e967690df7f757deaa..a69632a70cb48d88f3ca2b241c4924475dbe0088 100644 (file)
@@ -92,8 +92,9 @@ extern TransactionId GetOldestXmin(Relation rel, int flags);
 extern TransactionId GetOldestActiveTransactionId(void);
 extern TransactionId GetOldestSafeDecodingTransactionId(bool catalogOnly);
 
-extern VirtualTransactionId *GetVirtualXIDsDelayingChkpt(int *nvxids);
-extern bool HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids);
+extern VirtualTransactionId *GetVirtualXIDsDelayingChkpt(int *nvxids, int type);
+extern bool HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids,
+                                                                                int nvxids, int type);
 
 extern PGPROC *BackendPidGetProc(int pid);
 extern PGPROC *BackendPidGetProcWithLock(int pid);