Re: patch to allow disable of WAL recycling - Mailing list pgsql-hackers

At Thu, 19 Jul 2018 12:37:26 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20180719.123726.00899102.horiguchi.kyotaro@lab.ntt.co.jp>
> While considering this, I found a bug in 4b0d28de06, which
> removed prior checkpoint from control file. It actually trims the
> segments before the last checkpoint's redo segment but recycling
> is still considered based on the *prevous* checkpoint. As the
> result min_wal_size doesn't work as told.  Specifically, setting
> min/max_wal_size to 48MB and advance four or more segments then
> two checkpoints leaves just one segment, which is less than
> min_wal_size.
> 
> The attached patch fixes that. One arguable point on this would
> be the removal of the behavior when RemoveXLogFile(name,
> InvalidXLogRecPtr, ..).
> 
> The only place calling the function with the parameter is
> timeline switching. Previously unconditionally 10 segments are
> recycled after switchpoint but the reason for the behavior is we
> didn't have the information on previous checkpoint at hand at the
> time. But now we can use the timeline switch point as the
> approximate of the last checkpoint's redo point and this allows
> us to use min/max_wal_size properly at the time.

Fixed a comment in the patch, which was unreadable.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From f2b1a0b6360263d4ddf725075daf4b56800e3e18 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 19 Jul 2018 12:13:56 +0900
Subject: [PATCH] Fix calculation base of WAL recycling

The commit 4b0d28de06 removed the prior checkpoint and related things
but that leaves WAL recycling based on the prior checkpoint. This
makes max_wal_size and min_wal_size work incorrectly. This patch makes
WAL recycling be based on the last checkpoint.
---
 src/backend/access/transam/xlog.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 4049deb968..d7a61af8f1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2287,7 +2287,7 @@ assign_checkpoint_completion_target(double newval, void *extra)
  * XLOG segments? Returns the highest segment that should be preallocated.
  */
 static XLogSegNo
-XLOGfileslop(XLogRecPtr PriorRedoPtr)
+XLOGfileslop(XLogRecPtr RedoRecPtr)
 {
     XLogSegNo    minSegNo;
     XLogSegNo    maxSegNo;
@@ -2299,9 +2299,9 @@ XLOGfileslop(XLogRecPtr PriorRedoPtr)
      * correspond to. Always recycle enough segments to meet the minimum, and
      * remove enough segments to stay below the maximum.
      */
-    minSegNo = PriorRedoPtr / wal_segment_size +
+    minSegNo = RedoRecPtr / wal_segment_size +
         ConvertToXSegs(min_wal_size_mb, wal_segment_size) - 1;
-    maxSegNo = PriorRedoPtr / wal_segment_size +
+    maxSegNo = RedoRecPtr / wal_segment_size +
         ConvertToXSegs(max_wal_size_mb, wal_segment_size) - 1;
 
     /*
@@ -2316,7 +2316,7 @@ XLOGfileslop(XLogRecPtr PriorRedoPtr)
     /* add 10% for good measure. */
     distance *= 1.10;
 
-    recycleSegNo = (XLogSegNo) ceil(((double) PriorRedoPtr + distance) /
+    recycleSegNo = (XLogSegNo) ceil(((double) RedoRecPtr + distance) /
                                     wal_segment_size);
 
     if (recycleSegNo < minSegNo)
@@ -3896,12 +3896,12 @@ RemoveTempXlogFiles(void)
 /*
  * Recycle or remove all log files older or equal to passed segno.
  *
- * endptr is current (or recent) end of xlog, and PriorRedoRecPtr is the
- * redo pointer of the previous checkpoint. These are used to determine
+ * endptr is current (or recent) end of xlog, and RedoRecPtr 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 PriorRedoPtr, XLogRecPtr endptr)
+RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
 {
     DIR           *xldir;
     struct dirent *xlde;
@@ -3944,7 +3944,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, RedoRecPtr, endptr);
             }
         }
     }
@@ -4006,9 +4006,11 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint, TimeLineID newTLI)
              * remove it yet. It should be OK to remove it - files that are
              * not part of our timeline history are not required for recovery
              * - but seems safer to let them be archived and removed later.
+             * Here, switchpoint is a good approximate of RedoRecPtr for
+             * RemoveXlogFile since we have just done timeline switching.
              */
             if (!XLogArchiveIsReady(xlde->d_name))
-                RemoveXlogFile(xlde->d_name, InvalidXLogRecPtr, switchpoint);
+                RemoveXlogFile(xlde->d_name, switchpoint, switchpoint);
         }
     }
 
@@ -4018,14 +4020,12 @@ 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
+ * endptr is current (or recent) end of xlog, and RedoRecPtr 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 PriorRedoRecPtr is not known, pass invalid, and the function will
- * recycle, somewhat arbitrarily, 10 future segments.
  */
 static void
-RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr)
+RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
 {
     char        path[MAXPGPATH];
 #ifdef WIN32
@@ -4039,10 +4039,7 @@ RemoveXlogFile(const char *segname, XLogRecPtr PriorRedoPtr, XLogRecPtr endptr)
      * Initialize info about where to try to recycle to.
      */
     XLByteToSeg(endptr, endlogSegNo, wal_segment_size);
-    if (PriorRedoPtr == InvalidXLogRecPtr)
-        recycleSegNo = endlogSegNo + 10;
-    else
-        recycleSegNo = XLOGfileslop(PriorRedoPtr);
+    recycleSegNo = XLOGfileslop(RedoRecPtr);
 
     snprintf(path, MAXPGPATH, XLOGDIR "/%s", segname);
 
@@ -9057,7 +9054,7 @@ CreateCheckPoint(int flags)
         XLByteToSeg(RedoRecPtr, _logSegNo, wal_segment_size);
         KeepLogSeg(recptr, &_logSegNo);
         _logSegNo--;
-        RemoveOldXlogFiles(_logSegNo, PriorRedoPtr, recptr);
+        RemoveOldXlogFiles(_logSegNo, RedoRecPtr, recptr);
     }
 
     /*
@@ -9410,7 +9407,7 @@ CreateRestartPoint(int flags)
         if (RecoveryInProgress())
             ThisTimeLineID = replayTLI;
 
-        RemoveOldXlogFiles(_logSegNo, PriorRedoPtr, endptr);
+        RemoveOldXlogFiles(_logSegNo, RedoRecPtr, endptr);
 
         /*
          * Make more log segments if needed.  (Do this after recycling old log
-- 
2.16.3