Fix failure to guarantee that a checkpoint will write out pg_clog updates

for transaction commits that occurred just before the checkpoint.  This is
an EXTREMELY serious bug --- kudos to Satoshi Okada for creating a
reproducible test case to prove its existence.

1 files changed, 23 insertions, 3 deletions

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b3d51a49ec1..edbf9fb5d5c 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.115.2.1 2002/03/15 19:20:43 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.115.2.2 2004/08/11 04:09:12 tgl Exp $
  *
  * NOTES
  *		Transaction aborts can now occur two ways:
@@ -557,13 +557,27 @@ RecordTransactionCommit(void)
 	 */
 	if (MyXactMadeXLogEntry)
 	{
+		bool		madeTCentries;
 		XLogRecPtr	recptr;
 
 		BufmgrCommit();
 
 		START_CRIT_SECTION();
 
-		if (MyLastRecPtr.xrecoff != 0)
+		madeTCentries = (MyLastRecPtr.xrecoff != 0);
+
+		/*
+		 * We need to lock out checkpoint start between writing our XLOG
+		 * record and updating pg_clog.  Otherwise it is possible for the
+		 * checkpoint to set REDO after the XLOG record but fail to flush the
+		 * pg_clog update to disk, leading to loss of the transaction commit
+		 * if we crash a little later.  Slightly klugy fix for problem
+		 * discovered 2004-08-10.
+		 */
+		if (madeTCentries)
+			LWLockAcquire(CheckpointStartLock, LW_SHARED);
+
+		if (madeTCentries)
 		{
 			/* Need to emit a commit record */
 			XLogRecData rdata;
@@ -610,9 +624,13 @@ RecordTransactionCommit(void)
 		XLogFlush(recptr);
 
 		/* Mark the transaction committed in clog, if needed */
-		if (MyLastRecPtr.xrecoff != 0)
+		if (madeTCentries)
 			TransactionIdCommit(xid);
 
+		/* Unlock checkpoint lock if we acquired it */
+		if (madeTCentries)
+			LWLockRelease(CheckpointStartLock);
+
 		END_CRIT_SECTION();
 	}
 
@@ -712,6 +730,8 @@ RecordTransactionAbort(void)
 	 * nowhere in permanent storage, so no one will ever care if it
 	 * committed.)  We do not flush XLOG to disk in any case, since the
 	 * default assumption after a crash would be that we aborted, anyway.
+	 * For the same reason, we don't need to worry about interlocking
+	 * against checkpoint start.
 	 *
 	 * Extra check here is to catch case that we aborted partway through
 	 * RecordTransactionCommit ...