Remove XLogFileNameP() from the tree

XLogFileNameP() is a wrapper routine able to build a palloc'd string for
a WAL segment name, which is used for error string generation.  There
were several code paths where it gets called in a critical section,
where memory allocation is not allowed.  This results in triggering
an assertion failure instead of generating the wanted error message.

Another, more annoying, problem is that if the allocation to generate
the WAL segment name fails on OOM, then the failure would be escalated
to a PANIC.

This removes the routine and all its callers are replaced with a logic
using a fixed-size buffer.  This way, all the existing mistakes are
fixed and future ones are prevented.

Author: Masahiko Sawada
Reviewed-by: Michael Paquier, Álvaro Herrera
Discussion: https://postgr.es/m/CA+fd4k5gC9H4uoWMLg9K_QfNrnkkdEw+-AFveob9YX7z8JnKTA@mail.gmail.com

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5f0ee50092cb7adc91aacea62b210f61b1072944..ad0846842058c29963fadaa422eb604bf4f19f3b 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2499,14 +2499,21 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible)
                pgstat_report_wait_end();
                if (written <= 0)
                {
+                   char        xlogfname[MAXFNAMELEN];
+                   int         save_errno;
+
                    if (errno == EINTR)
                        continue;
+
+                   save_errno = errno;
+                   XLogFileName(xlogfname, ThisTimeLineID, openLogSegNo,
+                                wal_segment_size);
+                   errno = save_errno;
                    ereport(PANIC,
                            (errcode_for_file_access(),
                             errmsg("could not write to log file %s "
                                    "at offset %u, length %zu: %m",
-                                   XLogFileNameP(ThisTimeLineID, openLogSegNo),
-                                   startoffset, nleft)));
+                                   xlogfname, startoffset, nleft)));
                }
                nleft -= written;
                from += written;
@@ -3792,10 +3799,17 @@ XLogFileClose(void)
 #endif
 
    if (close(openLogFile) != 0)
+   {
+       char        xlogfname[MAXFNAMELEN];
+       int         save_errno = errno;
+
+       XLogFileName(xlogfname, ThisTimeLineID, openLogSegNo, wal_segment_size);
+       errno = save_errno;
        ereport(PANIC,
                (errcode_for_file_access(),
-                errmsg("could not close file \"%s\": %m",
-                       XLogFileNameP(ThisTimeLineID, openLogSegNo))));
+                errmsg("could not close file \"%s\": %m", xlogfname)));
+   }
+
    openLogFile = -1;
 }
 
@@ -5510,10 +5524,17 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
        fd = XLogFileInit(startLogSegNo, &use_existent, true);
 
        if (close(fd) != 0)
+       {
+           char        xlogfname[MAXFNAMELEN];
+           int         save_errno = errno;
+
+           XLogFileName(xlogfname, ThisTimeLineID, openLogSegNo,
+                        wal_segment_size);
+           errno = save_errno;
            ereport(ERROR,
                    (errcode_for_file_access(),
-                    errmsg("could not close file \"%s\": %m",
-                           XLogFileNameP(ThisTimeLineID, startLogSegNo))));
+                    errmsg("could not close file \"%s\": %m", xlogfname)));
+       }
    }
 
    /*
@@ -10079,10 +10100,19 @@ assign_xlog_sync_method(int new_sync_method, void *extra)
        {
            pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC_METHOD_ASSIGN);
            if (pg_fsync(openLogFile) != 0)
+           {
+               char        xlogfname[MAXFNAMELEN];
+               int         save_errno;
+
+               save_errno = errno;
+               XLogFileName(xlogfname, ThisTimeLineID, openLogSegNo,
+                            wal_segment_size);
+               errno = save_errno;
                ereport(PANIC,
                        (errcode_for_file_access(),
-                        errmsg("could not fsync file \"%s\": %m",
-                               XLogFileNameP(ThisTimeLineID, openLogSegNo))));
+                        errmsg("could not fsync file \"%s\": %m", xlogfname)));
+           }
+
            pgstat_report_wait_end();
            if (get_sync_bit(sync_method) != get_sync_bit(new_sync_method))
                XLogFileClose();
@@ -10100,32 +10130,25 @@ assign_xlog_sync_method(int new_sync_method, void *extra)
 void
 issue_xlog_fsync(int fd, XLogSegNo segno)
 {
+   char       *msg = NULL;
+
    pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC);
    switch (sync_method)
    {
        case SYNC_METHOD_FSYNC:
            if (pg_fsync_no_writethrough(fd) != 0)
-               ereport(PANIC,
-                       (errcode_for_file_access(),
-                        errmsg("could not fsync file \"%s\": %m",
-                               XLogFileNameP(ThisTimeLineID, segno))));
+               msg = _("could not fsync file \"%s\": %m");
            break;
 #ifdef HAVE_FSYNC_WRITETHROUGH
        case SYNC_METHOD_FSYNC_WRITETHROUGH:
            if (pg_fsync_writethrough(fd) != 0)
-               ereport(PANIC,
-                       (errcode_for_file_access(),
-                        errmsg("could not fsync write-through file \"%s\": %m",
-                               XLogFileNameP(ThisTimeLineID, segno))));
+               msg = _("could not fsync write-through file \"%s\": %m");
            break;
 #endif
 #ifdef HAVE_FDATASYNC
        case SYNC_METHOD_FDATASYNC:
            if (pg_fdatasync(fd) != 0)
-               ereport(PANIC,
-                       (errcode_for_file_access(),
-                        errmsg("could not fdatasync file \"%s\": %m",
-                               XLogFileNameP(ThisTimeLineID, segno))));
+               msg = _("could not fdatasync file \"%s\": %m");
            break;
 #endif
        case SYNC_METHOD_OPEN:
@@ -10136,19 +10159,22 @@ issue_xlog_fsync(int fd, XLogSegNo segno)
            elog(PANIC, "unrecognized wal_sync_method: %d", sync_method);
            break;
    }
-   pgstat_report_wait_end();
-}
 
-/*
- * Return the filename of given log segment, as a palloc'd string.
- */
-char *
-XLogFileNameP(TimeLineID tli, XLogSegNo segno)
-{
-   char       *result = palloc(MAXFNAMELEN);
+   /* PANIC if failed to fsync */
+   if (msg)
+   {
+       char        xlogfname[MAXFNAMELEN];
+       int         save_errno = errno;
 
-   XLogFileName(result, tli, segno, wal_segment_size);
-   return result;
+       XLogFileName(xlogfname, ThisTimeLineID, openLogSegNo,
+                    wal_segment_size);
+       errno = save_errno;
+       ereport(PANIC,
+               (errcode_for_file_access(),
+                errmsg(msg, xlogfname)));
+   }
+
+   pgstat_report_wait_end();
 }
 
 /*

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 446760ed6e7b771c4ae32b4abe1412f8fc464b8e..14efbf37d61c93666021d8f7e913dbed2a4879cb 100644 (file)
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -776,7 +776,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 
 /* openSegment callback for WALRead */
 static int
-wal_segment_open(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
+wal_segment_open(XLogSegNo nextSegNo, WALSegmentContext * segcxt,
                 TimeLineID *tli_p)
 {
    TimeLineID  tli = *tli_p;
@@ -944,7 +944,9 @@ void
 WALReadRaiseError(WALReadError *errinfo)
 {
    WALOpenSegment *seg = &errinfo->wre_seg;
-   char       *fname = XLogFileNameP(seg->ws_tli, seg->ws_segno);
+   char        fname[MAXFNAMELEN];
+
+   XLogFileName(fname, seg->ws_tli, seg->ws_segno, wal_segment_size);
 
    if (errinfo->wre_read < 0)
    {

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 72acb107670ca0ef5a071e81ebd4bfeeee406955..c1e439adb4065e1b8a52bc7e44e834977b01215b 100644 (file)
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -576,17 +576,17 @@ WalReceiverMain(void)
            char        xlogfname[MAXFNAMELEN];
 
            XLogWalRcvFlush(false);
+           XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
            if (close(recvFile) != 0)
                ereport(PANIC,
                        (errcode_for_file_access(),
                         errmsg("could not close log segment %s: %m",
-                               XLogFileNameP(recvFileTLI, recvSegNo))));
+                               xlogfname)));
 
            /*
             * Create .done file forcibly to prevent the streamed segment from
             * being archived later.
             */
-           XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
            if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS)
                XLogArchiveForceDone(xlogfname);
            else
@@ -900,6 +900,8 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
 
                XLogWalRcvFlush(false);
 
+               XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
+
                /*
                 * XLOG segment files will be re-read by recovery in startup
                 * process soon, so we don't advise the OS to release cache
@@ -909,13 +911,12 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
                    ereport(PANIC,
                            (errcode_for_file_access(),
                             errmsg("could not close log segment %s: %m",
-                                   XLogFileNameP(recvFileTLI, recvSegNo))));
+                                   xlogfname)));
 
                /*
                 * Create .done file forcibly to prevent the streamed segment
                 * from being archived later.
                 */
-               XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
                if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS)
                    XLogArchiveForceDone(xlogfname);
                else
@@ -943,11 +944,18 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
        if (recvOff != startoff)
        {
            if (lseek(recvFile, (off_t) startoff, SEEK_SET) < 0)
+           {
+               char        xlogfname[MAXFNAMELEN];
+               int         save_errno = errno;
+
+               XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
+               errno = save_errno;
                ereport(PANIC,
                        (errcode_for_file_access(),
                         errmsg("could not seek in log segment %s to offset %u: %m",
-                               XLogFileNameP(recvFileTLI, recvSegNo),
-                               startoff)));
+                               xlogfname, startoff)));
+           }
+
            recvOff = startoff;
        }
 
@@ -957,15 +965,21 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
        byteswritten = write(recvFile, buf, segbytes);
        if (byteswritten <= 0)
        {
+           char        xlogfname[MAXFNAMELEN];
+           int         save_errno;
+
            /* if write didn't set errno, assume no disk space */
            if (errno == 0)
                errno = ENOSPC;
+
+           save_errno = errno;
+           XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
+           errno = save_errno;
            ereport(PANIC,
                    (errcode_for_file_access(),
                     errmsg("could not write to log segment %s "
                            "at offset %u, length %lu: %m",
-                           XLogFileNameP(recvFileTLI, recvSegNo),
-                           recvOff, (unsigned long) segbytes)));
+                           xlogfname, recvOff, (unsigned long) segbytes)));
        }
 
        /* Update state for write */

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 8bafa65e50ad0317e2e7c607001f911e41bdf54d..8b2a2be1c077c13d2078f5f273c823fcde4962ef 100644 (file)
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2434,10 +2434,17 @@ WalSndSegmentOpen(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
     * too old WAL segment that has already been removed or recycled.
     */
    if (errno == ENOENT)
+   {
+       char        xlogfname[MAXFNAMELEN];
+       int         save_errno = errno;
+
+       XLogFileName(xlogfname, *tli_p, nextSegNo, wal_segment_size);
+       errno = save_errno;
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("requested WAL segment %s has already been removed",
-                       XLogFileNameP(*tli_p, nextSegNo))));
+                       xlogfname)));
+   }
    else
        ereport(ERROR,
                (errcode_for_file_access(),

diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index d519252aad77a37c1a2d7da98f4b93d7afdbb5a2..9b588c87a525675db8c7e2ecaf46c92ccab3577d 100644 (file)
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -288,7 +288,6 @@ extern bool RecoveryIsPaused(void);
 extern void SetRecoveryPause(bool recoveryPause);
 extern TimestampTz GetLatestXTime(void);
 extern TimestampTz GetCurrentChunkReplayStartTime(void);
-extern char *XLogFileNameP(TimeLineID tli, XLogSegNo segno);
 
 extern void UpdateControlFile(void);
 extern uint64 GetSystemIdentifier(void);

diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index e7e10beb4cd1d81e7f72d46db74d54fee80111f6..e295dc65fbee5b850230a2c457fad87fcf9cc074 100644 (file)
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -152,6 +152,10 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader;
 /* Length of XLog file name */
 #define XLOG_FNAME_LEN    24
 
+/*
+ * Generate a WAL segment file name.  Do not use this macro in a helper
+ * function allocating the result generated.
+ */
 #define XLogFileName(fname, tli, logSegNo, wal_segsz_bytes)    \
    snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli,       \
             (uint32) ((logSegNo) / XLogSegmentsPerXLogId(wal_segsz_bytes)), \