On Sun, Dec 08, 2019 at 02:14:03PM -0500, Tom Lane wrote: > That is, these arguments *used* to be a different LSN pointer, and that > commit changed them to be mostly equal to RedoRecPtr, and made what > seems like a not very well-advised renaming to go with that.
Indeed. That part was ill-thought. >> So it might make sense to remove the parameter from this >> function, too, and replace it with a flag parameter named >> something like "is_valid", or perhaps split the function >> into two functions, one for valid and one for invalid. > > Don't think I buy that. The fact that these arguments were until recently > different from RedoRecPtr suggests that they might someday be different > again, whereupon we'd have to laboriously revert such a parameter redesign. > I think I'd just go for names that don't have a hard implication that > the parameter values are the same as any particular global variable. Yeah, those APIs may have a slightly different meaning in the future, so I agree that it makes the most sense to rename the variables of the functions from RedoRecPtr to lastRedoPtr to outline the fact that we are referring to the redo LSN of the last checkpoint. Attached is a patch for that. I'd rather back-patch that to avoid any conflicts when working on bug fixes for stable branches. Thoughts? -- Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6bc1a6b46d..8e1f8612df 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -892,8 +892,8 @@ static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr);
static void XLogFileClose(void);
static void PreallocXlogFiles(XLogRecPtr endptr);
static void RemoveTempXlogFiles(void);
-static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr RedoRecPtr, XLogRecPtr endptr);
-static void RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr, XLogRecPtr endptr);
+static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr lastRedoPtr, XLogRecPtr endptr);
+static void RemoveXlogFile(const char *segname, XLogRecPtr lastRedoPtr, XLogRecPtr endptr);
static void UpdateLastRemovedPtr(char *filename);
static void ValidateXLOGDirectoryStructure(void);
static void CleanupBackupHistory(void);
@@ -2299,7 +2299,7 @@ assign_checkpoint_completion_target(double newval, void *extra)
* XLOG segments? Returns the highest segment that should be preallocated.
*/
static XLogSegNo
-XLOGfileslop(XLogRecPtr RedoRecPtr)
+XLOGfileslop(XLogRecPtr lastRedoPtr)
{
XLogSegNo minSegNo;
XLogSegNo maxSegNo;
@@ -2311,9 +2311,9 @@ XLOGfileslop(XLogRecPtr RedoRecPtr)
* correspond to. Always recycle enough segments to meet the minimum, and
* remove enough segments to stay below the maximum.
*/
- minSegNo = RedoRecPtr / wal_segment_size +
+ minSegNo = lastRedoPtr / wal_segment_size +
ConvertToXSegs(min_wal_size_mb, wal_segment_size) - 1;
- maxSegNo = RedoRecPtr / wal_segment_size +
+ maxSegNo = lastRedoPtr / wal_segment_size +
ConvertToXSegs(max_wal_size_mb, wal_segment_size) - 1;
/*
@@ -2328,7 +2328,7 @@ XLOGfileslop(XLogRecPtr RedoRecPtr)
/* add 10% for good measure. */
distance *= 1.10;
- recycleSegNo = (XLogSegNo) ceil(((double) RedoRecPtr + distance) /
+ recycleSegNo = (XLogSegNo) ceil(((double) lastRedoPtr + distance) /
wal_segment_size);
if (recycleSegNo < minSegNo)
@@ -3949,12 +3949,12 @@ RemoveTempXlogFiles(void)
/*
* Recycle or remove all log files older or equal to passed segno.
*
- * endptr is current (or recent) end of xlog, and RedoRecPtr is the
+ * endptr is current (or recent) end of xlog, and lastRedoPtr is the
* redo pointer of the last checkpoint. These are used to determine
* whether we want to recycle rather than delete no-longer-wanted log files.
*/
static void
-RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
+RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr lastRedoPtr, XLogRecPtr endptr)
{
DIR *xldir;
struct dirent *xlde;
@@ -3997,7 +3997,7 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
/* Update the last removed location in shared memory first */
UpdateLastRemovedPtr(xlde->d_name);
- RemoveXlogFile(xlde->d_name, RedoRecPtr, endptr);
+ RemoveXlogFile(xlde->d_name, lastRedoPtr, endptr);
}
}
}
@@ -4071,14 +4071,14 @@ 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 RedoRecPtr is the
+ * endptr is current (or recent) end of xlog, and lastRedoPtr is the
* redo pointer of the last checkpoint. These are used to determine
* whether we want to recycle rather than delete no-longer-wanted log files.
- * If RedoRecPtr is not known, pass invalid, and the function will recycle,
+ * If lastRedoPtr is not known, pass invalid, and the function will recycle,
* somewhat arbitrarily, 10 future segments.
*/
static void
-RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
+RemoveXlogFile(const char *segname, XLogRecPtr lastRedoPtr, XLogRecPtr endptr)
{
char path[MAXPGPATH];
#ifdef WIN32
@@ -4094,10 +4094,10 @@ RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
* Initialize info about where to try to recycle to.
*/
XLByteToSeg(endptr, endlogSegNo, wal_segment_size);
- if (RedoRecPtr == InvalidXLogRecPtr)
+ if (lastRedoPtr == InvalidXLogRecPtr)
recycleSegNo = endlogSegNo + 10;
else
- recycleSegNo = XLOGfileslop(RedoRecPtr);
+ recycleSegNo = XLOGfileslop(lastRedoPtr);
}
else
recycleSegNo = 0; /* keep compiler quiet */
signature.asc
Description: PGP signature
