Repair longstanding bug in slru/clog logic: it is possible for two backends

tglsfdc · tglsfdc · commit c89a0dd3bb2c · 2006-01-21T04:38:21.000Z
to try to create a log segment file concurrently, but the code erroneously
specified O_EXCL to open(), resulting in a needless failure.  Before 7.4,
it was even a PANIC condition :-(.  Correct code is actually simpler than
what we had, because we can just say O_CREAT to start with and not need a
second open() call.  I believe this accounts for several recent reports of
hard-to-reproduce "could not create file ...: File exists" errors in both
pg_clog and pg_subtrans.
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
@@ -41,7 +41,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.33 2005/12/06 23:08:32 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.34 2006/01/21 04:38:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -128,7 +128,6 @@ typedef struct SlruFlushData
 typedef enum
 {
 	SLRU_OPEN_FAILED,
-	SLRU_CREATE_FAILED,
 	SLRU_SEEK_FAILED,
 	SLRU_READ_FAILED,
 	SLRU_WRITE_FAILED,
@@ -652,26 +651,19 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
 		 * transactions that have already been truncated from the commit log.
 		 * Easiest way to deal with that is to accept references to
 		 * nonexistent files here and in SlruPhysicalReadPage.)
+		 *
+		 * Note: it is possible for more than one backend to be executing
+		 * this code simultaneously for different pages of the same file.
+		 * Hence, don't use O_EXCL or O_TRUNC or anything like that.
 		 */
 		SlruFileName(ctl, path, segno);
-		fd = BasicOpenFile(path, O_RDWR | PG_BINARY, S_IRUSR | S_IWUSR);
+		fd = BasicOpenFile(path, O_RDWR | O_CREAT | PG_BINARY,
+						   S_IRUSR | S_IWUSR);
 		if (fd < 0)
 		{
-			if (errno != ENOENT)
-			{
-				slru_errcause = SLRU_OPEN_FAILED;
-				slru_errno = errno;
-				return false;
-			}
-
-			fd = BasicOpenFile(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
-							   S_IRUSR | S_IWUSR);
-			if (fd < 0)
-			{
-				slru_errcause = SLRU_CREATE_FAILED;
-				slru_errno = errno;
-				return false;
-			}
+			slru_errcause = SLRU_OPEN_FAILED;
+			slru_errno = errno;
+			return false;
 		}
 
 		if (fdata)
@@ -763,13 +755,6 @@ SlruReportIOError(SlruCtl ctl, int pageno, TransactionId xid)
 					 errdetail("could not open file \"%s\": %m",
 							   path)));
 			break;
-		case SLRU_CREATE_FAILED:
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not access status of transaction %u", xid),
-					 errdetail("could not create file \"%s\": %m",
-							   path)));
-			break;
 		case SLRU_SEEK_FAILED:
 			ereport(ERROR,
 					(errcode_for_file_access(),
