Date: Mon, 18 May 2020 19:44:59 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, jgdr@dalibo.com,
	michael@paquier.xyz, sawada.mshk@gmail.com,
	peter.eisentraut@2ndquadrant.com,
	pgsql-hackers@lists.postgresql.org, thomas.munro@enterprisedb.com,
	sk@zsrv.org, michael.paquier@gmail.com
Subject: Re: [HACKERS] Restricting maximum keep segments by repslots
Message-ID: <20200518234459.GA1850@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="mYCpIKhGyMATD0i+"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200517025150.GA12478@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--mYCpIKhGyMATD0i+
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

BTW while you're messing with checkpointer, I propose this patch to
simplify things.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--mYCpIKhGyMATD0i+
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-CreateCheckPoint-return-bool.patch"

From 9148a6defa2e8b3fd81b982de53f73584a8b3d10 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 8 May 2020 15:28:57 -0400
Subject: [PATCH] CreateCheckPoint return bool

---
 src/backend/access/transam/xlog.c     | 13 +++++++++++--
 src/backend/postmaster/checkpointer.c |  9 ++-------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ca09d81b08..8990ef7348 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8695,8 +8695,10 @@ UpdateCheckPointDistanceEstimate(uint64 nbytes)
  * All of this mechanism allows us to continue working while we checkpoint.
  * As a result, timing of actions is critical here and be careful to note that
  * this function will likely take minutes to execute on a busy system.
+ *
+ * Return value is true.
  */
-void
+bool
 CreateCheckPoint(int flags)
 {
 	bool		shutdown;
@@ -8815,7 +8817,12 @@ CreateCheckPoint(int flags)
 			END_CRIT_SECTION();
 			ereport(DEBUG1,
 					(errmsg("checkpoint skipped because system is idle")));
-			return;
+
+			/*
+			 * Returns true even if checkpoint is skipped; this is required to
+			 * prevent breaking the checkpoint scheduling algorithm.
+			 */
+			return true;
 		}
 	}
 
@@ -9107,6 +9114,8 @@ CreateCheckPoint(int flags)
 									 CheckpointStats.ckpt_segs_recycled);
 
 	LWLockRelease(CheckpointLock);
+
+	return true;
 }
 
 /*
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 34ed9f7887..380b243547 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -436,13 +436,8 @@ CheckpointerMain(void)
 			/*
 			 * Do the checkpoint.
 			 */
-			if (!do_restartpoint)
-			{
-				CreateCheckPoint(flags);
-				ckpt_performed = true;
-			}
-			else
-				ckpt_performed = CreateRestartPoint(flags);
+			ckpt_performed = do_restartpoint ?
+				CreateRestartPoint(flags) : CreateCheckPoint(flags);
 
 			/*
 			 * After any checkpoint, close all smgr files.  This is so we
-- 
2.20.1


--mYCpIKhGyMATD0i+--