Don't ERROR on PreallocXlogFiles() race condition.

Before a restartpoint finishes PreallocXlogFiles(), a startup process
KeepFileRestoredFromArchive() call can unlink the preallocated segment.
If a CHECKPOINT sql command had elicited the restartpoint experiencing
the race condition, that sql command failed.  Moreover, the restartpoint
omitted its log_checkpoints message and some inessential resource
reclamation.  Prevent the ERROR by skipping open() of the segment.
Since these consequences are so minor, no back-patch.

Discussion: https://postgr.es/m/20210202151416[email protected]

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1a31788071cf47a05aff2404eb50fd64c19a30b0..8cda20d803d5694feda64e69eb56e9e9eeca99c3 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2424,7 +2424,6 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
    bool        ispartialpage;
    bool        last_iteration;
    bool        finishing_seg;
-   bool        added;
    int         curridx;
    int         npages;
    int         startidx;
@@ -2490,7 +2489,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
                            wal_segment_size);
 
            /* create/use new log file */
-           openLogFile = XLogFileInit(openLogSegNo, &added);
+           openLogFile = XLogFileInit(openLogSegNo);
            ReserveExternalFD();
        }
 
@@ -3255,23 +3254,21 @@ XLogNeedsFlush(XLogRecPtr record)
 }
 
 /*
- * Create a new XLOG file segment, or open a pre-existing one.
+ * Try to make a given XLOG file segment exist.
  *
- * logsegno: identify segment to be created/opened.
+ * logsegno: identify segment.
  *
  * *added: on return, true if this call raised the number of extant segments.
  *
- * Returns FD of opened file.
+ * path: on return, this char[MAXPGPATH] has the path to the logsegno file.
  *
- * Note: errors here are ERROR not PANIC because we might or might not be
- * inside a critical section (eg, during checkpoint there is no reason to
- * take down the system on failure).  They will promote to PANIC if we are
- * in a critical section.
+ * Returns -1 or FD of opened file.  A -1 here is not an error; a caller
+ * wanting an open segment should attempt to open "path", which usually will
+ * succeed.  (This is weird, but it's efficient for the callers.)
  */
-int
-XLogFileInit(XLogSegNo logsegno, bool *added)
+static int
+XLogFileInitInternal(XLogSegNo logsegno, bool *added, char *path)
 {
-   char        path[MAXPGPATH];
    char        tmppath[MAXPGPATH];
    PGAlignedXLogBlock zbuffer;
    XLogSegNo   installed_segno;
@@ -3424,26 +3421,53 @@ XLogFileInit(XLogSegNo logsegno, bool *added)
     */
    max_segno = logsegno + CheckPointSegments;
    if (InstallXLogFileSegment(&installed_segno, tmppath, true, max_segno))
+   {
        *added = true;
+       elog(DEBUG2, "done creating and filling new WAL file");
+   }
    else
    {
        /*
         * No need for any more future segments, or InstallXLogFileSegment()
-        * failed to rename the file into place. If the rename failed, opening
-        * the file below will fail.
+        * failed to rename the file into place. If the rename failed, a
+        * caller opening the file may fail.
         */
        unlink(tmppath);
+       elog(DEBUG2, "abandoned new WAL file");
    }
 
+   return -1;
+}
+
+/*
+ * Create a new XLOG file segment, or open a pre-existing one.
+ *
+ * logsegno: identify segment to be created/opened.
+ *
+ * Returns FD of opened file.
+ *
+ * Note: errors here are ERROR not PANIC because we might or might not be
+ * inside a critical section (eg, during checkpoint there is no reason to
+ * take down the system on failure).  They will promote to PANIC if we are
+ * in a critical section.
+ */
+int
+XLogFileInit(XLogSegNo logsegno)
+{
+   bool        ignore_added;
+   char        path[MAXPGPATH];
+   int         fd;
+
+   fd = XLogFileInitInternal(logsegno, &ignore_added, path);
+   if (fd >= 0)
+       return fd;
+
    /* Now open original target segment (might not be file I just made) */
    fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
    if (fd < 0)
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not open file \"%s\": %m", path)));
-
-   elog(DEBUG2, "done creating and filling new WAL file");
-
    return fd;
 }
 
@@ -3903,6 +3927,15 @@ XLogFileClose(void)
  * High-volume systems will be OK once they've built up a sufficient set of
  * recycled log segments, but the startup transient is likely to include
  * a lot of segment creations by foreground processes, which is not so good.
+ *
+ * XLogFileInitInternal() can ereport(ERROR).  All known causes indicate big
+ * trouble; for example, a full filesystem is one cause.  The checkpoint WAL
+ * and/or ControlFile updates already completed.  If a RequestCheckpoint()
+ * initiated the present checkpoint and an ERROR ends this function, the
+ * command that called RequestCheckpoint() fails.  That's not ideal, but it's
+ * not worth contorting more functions to use caller-specified elevel values.
+ * (With or without RequestCheckpoint(), an ERROR forestalls some inessential
+ * reporting and resource reclamation.)
  */
 static void
 PreallocXlogFiles(XLogRecPtr endptr)
@@ -3910,6 +3943,7 @@ PreallocXlogFiles(XLogRecPtr endptr)
    XLogSegNo   _logSegNo;
    int         lf;
    bool        added;
+   char        path[MAXPGPATH];
    uint64      offset;
 
    XLByteToPrevSeg(endptr, _logSegNo, wal_segment_size);
@@ -3917,8 +3951,9 @@ PreallocXlogFiles(XLogRecPtr endptr)
    if (offset >= (uint32) (0.75 * wal_segment_size))
    {
        _logSegNo++;
-       lf = XLogFileInit(_logSegNo, &added);
-       close(lf);
+       lf = XLogFileInitInternal(_logSegNo, &added, path);
+       if (lf >= 0)
+           close(lf);
        if (added)
            CheckpointStats.ckpt_segs_added++;
    }
@@ -5214,7 +5249,6 @@ BootStrapXLOG(void)
    XLogLongPageHeader longpage;
    XLogRecord *record;
    char       *recptr;
-   bool        added;
    uint64      sysidentifier;
    struct timeval tv;
    pg_crc32c   crc;
@@ -5311,7 +5345,7 @@ BootStrapXLOG(void)
    record->xl_crc = crc;
 
    /* Create first XLOG segment file */
-   openLogFile = XLogFileInit(1, &added);
+   openLogFile = XLogFileInit(1);
 
    /*
     * We needn't bother with Reserve/ReleaseExternalFD here, since we'll
@@ -5617,10 +5651,9 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
         * The switch happened at a segment boundary, so just create the next
         * segment on the new timeline.
         */
-       bool        added;
        int         fd;
 
-       fd = XLogFileInit(startLogSegNo, &added);
+       fd = XLogFileInit(startLogSegNo);
 
        if (close(fd) != 0)
        {

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index eadff8f908fe60a82270fe2ab8e5ff927f696e80..2be9ad967dc20aa6311943415be47d60ea4f1fd2 100644 (file)
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -885,8 +885,6 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
 
        if (recvFile < 0 || !XLByteInSeg(recptr, recvSegNo, wal_segment_size))
        {
-           bool        added;
-
            /*
             * fsync() and close current file before we switch to next one. We
             * would otherwise have to reopen this file to fsync it later
@@ -923,7 +921,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
 
            /* Create/use new log file */
            XLByteToSeg(recptr, recvSegNo, wal_segment_size);
-           recvFile = XLogFileInit(recvSegNo, &added);
+           recvFile = XLogFileInit(recvSegNo);
            recvFileTLI = ThisTimeLineID;
        }
 

diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index d8b8f59c4bd79a16e4d3d6de037dfd511fa52e5b..7510e882287207a6f230777b0c104d2bb592713c 100644 (file)
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -296,7 +296,7 @@ extern XLogRecPtr XLogInsertRecord(struct XLogRecData *rdata,
 extern void XLogFlush(XLogRecPtr RecPtr);
 extern bool XLogBackgroundFlush(void);
 extern bool XLogNeedsFlush(XLogRecPtr RecPtr);
-extern int XLogFileInit(XLogSegNo segno, bool *added);
+extern int XLogFileInit(XLogSegNo segno);
 extern int XLogFileOpen(XLogSegNo segno);
 
 extern void CheckXLogRemoved(XLogSegNo segno, TimeLineID tli);