On Tue, Jun 9, 2015 at 6:25 AM, Heikki Linnakangas wrote:
> I'm still not sure if I should've just reverted that refactoring, to make
> XLogFileCopy() look the same in master and back-branches, which makes
> back-patching easier, or keep the refactoring, because it makes the code
> slightly nicer. But the current situation is the worst of both worlds: the
> interface of XLogFileCopy() is no better than it used to be, but it's
> different enough to cause merge conflicts. At this point, it's probably best
> to revert the code to look the same as in 9.4.
That's a valid concern. What about the attached then? I think that it
is still good to keep upto to copy only data up to the switch point at
recovery exit. InstallXLogFileSegment() changes a bit as well because
of its modifications of arguments.
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 150d56a..e8d3524 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, int elevel);
+ bool use_lock);
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, LOG))
+ use_lock))
{
/*
* No need for any more future segments, or InstallXLogFileSegment()
@@ -3039,20 +3039,25 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
}
/*
- * Copy a WAL segment file in pg_xlog directory.
+ * Create a new XLOG file segment by copying a pre-existing one.
*
- * srcfname source filename
- * upto how much of the source file to copy? (the rest is filled with
- * zeros)
- * segno identify segment to install.
+ * destsegno: identify segment to be created.
*
- * The file is first copied with a temporary filename, and then installed as
- * a newly-created segment.
+ * srcTLI, srclog, srcseg: identify segment to be copied (could be from
+ * a different timeline)
+ *
+ * upto: how much of the source file to copy (the rest is filled with
+ * zeros)
+ *
+ * Currently this is only used during recovery, and so there are no locking
+ * considerations. But we should be just as tense as XLogFileInit to avoid
+ * emplacing a bogus file.
*/
static void
-XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
+XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
+ int upto)
{
- char srcpath[MAXPGPATH];
+ char path[MAXPGPATH];
char tmppath[MAXPGPATH];
char buffer[XLOG_BLCKSZ];
int srcfd;
@@ -3062,12 +3067,12 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
/*
* Open the source file
*/
- snprintf(srcpath, MAXPGPATH, XLOGDIR "/%s", srcfname);
- srcfd = OpenTransientFile(srcpath, O_RDONLY | PG_BINARY, 0);
+ XLogFilePath(path, srcTLI, srcsegno);
+ srcfd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
if (srcfd < 0)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not open file \"%s\": %m", srcpath)));
+ errmsg("could not open file \"%s\": %m", path)));
/*
* Copy into a temp file name.
@@ -3111,11 +3116,11 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read file \"%s\": %m",
- srcpath)));
+ path)));
else
ereport(ERROR,
(errmsg("not enough data in file \"%s\"",
- srcpath)));
+ path)));
}
}
errno = 0;
@@ -3148,9 +3153,11 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
CloseTransientFile(srcfd);
- /* install the new file */
- (void) InstallXLogFileSegment(&segno, tmppath, false,
- 0, false, ERROR);
+ /*
+ * Now move the segment into place with its final name.
+ */
+ if (!InstallXLogFileSegment(&destsegno, tmppath, false, 0, false))
+ elog(ERROR, "InstallXLogFileSegment should not have failed");
}
/*
@@ -3177,8 +3184,6 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
* 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.
@@ -3186,7 +3191,7 @@ XLogFileCopy(char *srcfname, int upto, XLogSegNo segno)
static bool
InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
bool find_free, XLogSegNo max_segno,
- bool use_lock, int elevel)
+ bool use_lock)
{
char path[MAXPGPATH];
struct stat stat_buf;
@@ -3231,7 +3236,7 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
{
if (use_lock)
LWLockRelease(ControlFileLock);
- ereport(elevel,
+ ereport(LOG,
(errcode_for_file_access(),
errmsg("could not link file \"%s\" to \"%s\" (initialization of log file): %m",
tmppath, path)));
@@ -3243,7 +3248,7 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
{
if (use_lock)
LWLockRelease(ControlFileLock);
- ereport(elevel,
+ ereport(LOG,
(errcode_for_file_access(),
errmsg("could not rename file \"%s\" to \"%s\" (initialization of log file): %m",
tmppath, path)));
@@ -3732,7 +3737,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, LOG))
+ true, recycleSegNo, true))
{
ereport(DEBUG2,
(errmsg("recycled transaction log file \"%s\"",
@@ -5211,8 +5216,6 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
*/
if (endLogSegNo == startLogSegNo)
{
- XLogFileName(xlogfname, endTLI, endLogSegNo);
-
/*
* Make a copy of the file on the new timeline.
*
@@ -5220,7 +5223,8 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
* considerations. But we should be just as tense as XLogFileInit to
* avoid emplacing a bogus file.
*/
- XLogFileCopy(xlogfname, endOfLog % XLOG_SEG_SIZE, endLogSegNo);
+ XLogFileCopy(endLogSegNo, endTLI, endLogSegNo,
+ endOfLog % XLOG_SEG_SIZE);
}
else
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
