Refactor WAL segment copying code.

* Remove unused argument "dstfname" and related code from XLogFileCopy().

* Previously XLogFileCopy() returned a pstrdup'd string so that
InstallXLogFileSegment() used it later. Since the pstrdup'd string was never
free'd, there could be a risk of memory leak. It was almost harmless because
the startup process exited just after calling XLogFileCopy(), it existed.
This commit changes XLogFileCopy() so that it directly calls
InstallXLogFileSegment() and doesn't call pstrdup() at all. Which fixes that
memory leak problem.

* Extend InstallXLogFileSegment() so that the caller can specify the log level.
Which allows us to emit an error when InstallXLogFileSegment() fails a disk
file access like link() and rename(). Previously it was always logged with
LOG level and additionally needed to be logged with ERROR when we wanted
to treat it as an error.

Michael Paquier

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b732a9d76f7360d4a7b323f9874f37c74a256f9a..5bebf99c3978c42a7a70bed4e913da165ecb2e73 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -807,7 +807,7 @@ static bool XLogCheckpointNeeded(XLogSegNo new_segno);
 static void XLogWrite(XLogwrtRqst WriteRqst, bool flexible);
 static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
                                           bool find_free, XLogSegNo max_segno,
-                                          bool use_lock);
+                                          bool use_lock, int elevel);
 static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
                         int source, bool notexistOk);
 static int     XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
@@ -3012,7 +3012,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
        max_segno = logsegno + CheckPointSegments;
        if (!InstallXLogFileSegment(&installed_segno, tmppath,
                                                                *use_existent, max_segno,
-                                                               use_lock))
+                                                               use_lock, LOG))
        {
                /*
                 * No need for any more future segments, or InstallXLogFileSegment()
@@ -3041,18 +3041,16 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
 /*
  * Copy a WAL segment file in pg_xlog directory.
  *
- * dstfname            destination filename
  * srcfname            source filename
  * upto                        how much of the source file to copy? (the rest is filled with
  *                             zeros)
+ * segno               identify segment to install.
  *
- * If dstfname is not given, the file is created with a temporary filename,
- * which is returned.  Both filenames are relative to the pg_xlog directory.
- *
- * NB: Any existing file with the same name will be overwritten!
+ * The file is first copied with a temporary filename, and then installed as
+ * a newly-created segment.
  */
-static char *
-XLogFileCopy(char *dstfname, char *srcfname, int upto)
+static void
+XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
 {
        char            srcpath[MAXPGPATH];
        char            tmppath[MAXPGPATH];
@@ -3150,25 +3148,9 @@ XLogFileCopy(char *dstfname, char *srcfname, int upto)
 
        CloseTransientFile(srcfd);
 
-       /*
-        * Now move the segment into place with its final name.  (Or just return
-        * the path to the file we created, if the caller wants to handle the rest
-        * on its own.)
-        */
-       if (dstfname)
-       {
-               char            dstpath[MAXPGPATH];
-
-               snprintf(dstpath, MAXPGPATH, XLOGDIR "/%s", dstfname);
-               if (rename(tmppath, dstpath) < 0)
-                       ereport(ERROR,
-                                       (errcode_for_file_access(),
-                                        errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                       tmppath, dstpath)));
-               return NULL;
-       }
-       else
-               return pstrdup(tmppath);
+       /* install the new file */
+       (void) InstallXLogFileSegment(&segno, tmppath, false,
+                                                                 0, false, ERROR);
 }
 
 /*
@@ -3195,6 +3177,8 @@ XLogFileCopy(char *dstfname, char *srcfname, int upto)
  * place.  This should be TRUE except during bootstrap log creation.  The
  * caller must *not* hold the lock at call.
  *
+ * elevel: log level used by this routine.
+ *
  * Returns TRUE if the file was installed successfully.  FALSE indicates that
  * max_segno limit was exceeded, or an error occurred while renaming the
  * file into place.
@@ -3202,7 +3186,7 @@ XLogFileCopy(char *dstfname, char *srcfname, int upto)
 static bool
 InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
                                           bool find_free, XLogSegNo max_segno,
-                                          bool use_lock)
+                                          bool use_lock, int elevel)
 {
        char            path[MAXPGPATH];
        struct stat stat_buf;
@@ -3247,7 +3231,7 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
        {
                if (use_lock)
                        LWLockRelease(ControlFileLock);
-               ereport(LOG,
+               ereport(elevel,
                                (errcode_for_file_access(),
                                 errmsg("could not link file \"%s\" to \"%s\" (initialization of log file): %m",
                                                tmppath, path)));
@@ -3259,7 +3243,7 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
        {
                if (use_lock)
                        LWLockRelease(ControlFileLock);
-               ereport(LOG,
+               ereport(elevel,
                                (errcode_for_file_access(),
                                 errmsg("could not rename file \"%s\" to \"%s\" (initialization of log file): %m",
                                                tmppath, path)));
@@ -3748,7 +3732,7 @@ RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr)
        if (endlogSegNo <= recycleSegNo &&
                lstat(path, &statbuf) == 0 && S_ISREG(statbuf.st_mode) &&
                InstallXLogFileSegment(&endlogSegNo, path,
-                                                          true, recycleSegNo, true))
+                                                          true, recycleSegNo, true, LOG))
        {
                ereport(DEBUG2,
                                (errmsg("recycled transaction log file \"%s\"",
@@ -5227,8 +5211,6 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
         */
        if (endLogSegNo == startLogSegNo)
        {
-               char       *tmpfname;
-
                XLogFileName(xlogfname, endTLI, endLogSegNo);
 
                /*
@@ -5238,9 +5220,7 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
                 * considerations. But we should be just as tense as XLogFileInit to
                 * avoid emplacing a bogus file.
                 */
-               tmpfname = XLogFileCopy(NULL, xlogfname, endOfLog % XLOG_SEG_SIZE);
-               if (!InstallXLogFileSegment(&endLogSegNo, tmpfname, false, 0, false))
-                       elog(ERROR, "InstallXLogFileSegment should not have failed");
+               XLogFileCopy(xlogfname, endOfLog % XLOG_SEG_SIZE, endLogSegNo);
        }
        else
        {