From 7abc68597436da1475b4d9b08f4fa9f3c5ed6185 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Tue, 9 Jun 2015 03:03:24 +0900
Subject: [PATCH] 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
---
src/backend/access/transam/xlog.c | 54 ++++++++++---------------------
1 file changed, 17 insertions(+), 37 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b732a9d76f7..5bebf99c397 100644
--- 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
{
--
GitLab