On Thu, Jun 22, 2017 at 6:10 AM, Andres Freund <and...@anarazel.de> wrote: > We've not heard any complaints about this afaik, but it's not something > that's easily diagnosable as being a problem. Therefore I suspect we > should fix and backpatch this?
Agreed. I have just poked at this problem and have finished with the attached. Logically it is not complicated at the argument values used by the callers of RemoveXlogFile() are never updated when scanning pg_wal. Surely this patch needs an extra pair of eyes. -- Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0a6314a642..d594401b6e 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -878,7 +878,8 @@ static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr); static void XLogFileClose(void); static void PreallocXlogFiles(XLogRecPtr endptr); static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr); -static void RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr); +static void RemoveXlogFile(const char *segname, XLogSegNo *endlogSegNo, + XLogSegNo recycleSegNo); static void UpdateLastRemovedPtr(char *filename); static void ValidateXLOGDirectoryStructure(void); static void CleanupBackupHistory(void); @@ -3829,10 +3830,18 @@ UpdateLastRemovedPtr(char *filename) static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr) { + XLogSegNo endlogSegNo; + XLogSegNo recycleSegNo; DIR *xldir; struct dirent *xlde; char lastoff[MAXFNAMELEN]; + /* + * Initialize info about where to try to recycle to. + */ + XLByteToPrevSeg(endptr, endlogSegNo); + recycleSegNo = XLOGfileslop(PriorRedoPtr); + xldir = AllocateDir(XLOGDIR); if (xldir == NULL) ereport(ERROR, @@ -3875,7 +3884,7 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr) /* Update the last removed location in shared memory first */ UpdateLastRemovedPtr(xlde->d_name); - RemoveXlogFile(xlde->d_name, PriorRedoPtr, endptr); + RemoveXlogFile(xlde->d_name, &endlogSegNo, recycleSegNo); } } } @@ -3905,8 +3914,14 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI) struct dirent *xlde; char switchseg[MAXFNAMELEN]; XLogSegNo endLogSegNo; + XLogSegNo recycleSegNo; + /* + * Initialize info about where to begin the work. This will recycle + * arbitrarily 10 segments. + */ XLByteToPrevSeg(switchpoint, endLogSegNo); + recycleSegNo = endLogSegNo + 10; xldir = AllocateDir(XLOGDIR); if (xldir == NULL) @@ -3944,7 +3959,7 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI) * - but seems safer to let them be archived and removed later. */ if (!XLogArchiveIsReady(xlde->d_name)) - RemoveXlogFile(xlde->d_name, InvalidXLogRecPtr, switchpoint); + RemoveXlogFile(xlde->d_name, &endLogSegNo, recycleSegNo); } } @@ -3954,31 +3969,22 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI) /* * Recycle or remove a log file that's no longer needed. * - * endptr is current (or recent) end of xlog, and PriorRedoRecPtr is the - * redo pointer of the previous checkpoint. These are used to determine - * whether we want to recycle rather than delete no-longer-wanted log files. - * If PriorRedoRecPtr is not known, pass invalid, and the function will - * recycle, somewhat arbitrarily, 10 future segments. + * segname is the name of the segment to recycle or remove. endlogSegNo + * is the segment number of the current (or recent) end of WAL. recycleSegNo + * is the segment number to recycle up to. + * + * endlogSegNo gets incremented if the segment is recycled so as it is not + * checked again with future callers of this function. */ static void -RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr) +RemoveXlogFile(const char *segname, XLogSegNo *endlogSegNo, + XLogSegNo recycleSegNo) { char path[MAXPGPATH]; #ifdef WIN32 char newpath[MAXPGPATH]; #endif struct stat statbuf; - XLogSegNo endlogSegNo; - XLogSegNo recycleSegNo; - - /* - * Initialize info about where to try to recycle to. - */ - XLByteToSeg(endptr, endlogSegNo); - if (PriorRedoPtr == InvalidXLogRecPtr) - recycleSegNo = endlogSegNo + 10; - else - recycleSegNo = XLOGfileslop(PriorRedoPtr); snprintf(path, MAXPGPATH, XLOGDIR "/%s", segname); @@ -3987,9 +3993,9 @@ RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr) * segment. Only recycle normal files, pg_standby for example can create * symbolic links pointing to a separate archive directory. */ - if (endlogSegNo <= recycleSegNo && + if (*endlogSegNo <= recycleSegNo && lstat(path, &statbuf) == 0 && S_ISREG(statbuf.st_mode) && - InstallXLogFileSegment(&endlogSegNo, path, + InstallXLogFileSegment(endlogSegNo, path, true, recycleSegNo, true)) { ereport(DEBUG2, @@ -3997,7 +4003,7 @@ RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr) segname))); CheckpointStats.ckpt_segs_recycled++; /* Needn't recheck that slot on future iterations */ - endlogSegNo++; + (*endlogSegNo)++; } else {
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers