On Mon, Dec 09, 2019 at 05:11:10PM -0500, Tom Lane wrote: > Alvaro Herrera <alvhe...@2ndquadrant.com> writes: >> We have a not-consistently-used convention that names in CamelCase are >> used for global variables. Naming a function parameter in that style >> seems pointlessly confusing. I would rather use redorecptr as Tom >> suggested, which fits with the style used for the other arguments of >> that function. BTW prepending an X or a p looks like minimum effort... >> I'd stay away from that. > > Actually, for the particular case of RemoveXlogFile(s), I wonder if it > shouldn't be "startptr" to go with the other argument "endptr". This line > of thinking might not lead to nicer names in other functions, of course. > But we shouldn't assume that a one-size-fits-all solution is going to > improve legibility, and in the end, legibility is what this should be > about.
Hmm. In the case of this logic, we are referring to the current end of WAL with endptr, and what you are calling the startptr is really the redo LSN of the last checkpoint in all the routines which are now confused with RedoRecPtr: RemoveOldXlogFile, RemoveXlogFile and XLOGfileslop. Using lower-case for all the characters of the variable name sounds like a good improvement as well, so taking a combination of all that I would just use "lastredoptr" in those three code paths (note that we used to have PriorRedoPtr before). As that's a confusion I introduced with d9fadbf, I would like to fix that and backpatch this change down to 11. (Ranier gets the authorship per se as that's extracted from a larger patch). -- Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3baf1b009a..71b8389ba1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -891,8 +891,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);
@@ -2298,7 +2298,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;
@@ -2310,9 +2310,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;
/*
@@ -2327,7 +2327,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)
@@ -3948,12 +3948,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;
@@ -3996,7 +3996,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);
}
}
}
@@ -4070,14 +4070,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
@@ -4093,10 +4093,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
