postgres.git / summary / log / commit / refs

commit    e6c44eef55cda493c759e926cecceb92186159b8
Author:   Andres Freund <andres@anarazel.de>
Date:     Sat May 06 23:47:40 2017 +0000
First released in: 10.0

    Fix off-by-one possibly leading to skipped XLOG_RUNNING_XACTS records.
    
    Since 6ef2eba3f57f1 ("Skip checkpoints, archiving on idle systems."),
    GetLastImportantRecPtr() is used to avoid performing superfluous
    checkpoints, xlog switches, running-xact records when the system is
    idle.  Unfortunately the check concerning running-xact records had a
    off-by-one error, leading to such records being potentially skipped
    when only a single record has been inserted since the last
    running-xact record.
    
    An alternative approach would have been to change
    GetLastImportantRecPtr()'s definition to point to the end of records,
    but that would make the checkpoint code more complicated.
    
    Author: Andres Freund
    Discussion: https://postgr.es/m/20170505012447.wsrympaxnfis6ojt@alap3.anarazel.de
    Backpatch: no, code only present in master


 src/backend/postmaster/bgwriter.c     | 7 +++++--
 src/backend/postmaster/checkpointer.c | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index dcb4cf249c5..48efe15e825 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -325,10 +325,13 @@ BackgroundWriterMain(void)
 
 			/*
 			 * Only log if enough time has passed and interesting records have
-			 * been inserted since the last snapshot.
+			 * been inserted since the last snapshot.  Have to compare with <=
+			 * instead of < because GetLastImportantRecPtr() points at the
+			 * start of a record, whereas last_snapshot_lsn points just past
+			 * the end of the record.
 			 */
 			if (now >= timeout &&
-				last_snapshot_lsn < GetLastImportantRecPtr())
+				last_snapshot_lsn <= GetLastImportantRecPtr())
 			{
 				last_snapshot_lsn = LogStandbySnapshot();
 				last_snapshot_ts = now;
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index fe9041f748c..d12db0d5a7b 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -611,7 +611,8 @@ CheckArchiveTimeout(void)
 	{
 		/*
 		 * Switch segment only when "important" WAL has been logged since the
-		 * last segment switch.
+		 * last segment switch (last_switch_lsn points to end of segment
+		 * switch occurred in).
 		 */
 		if (GetLastImportantRecPtr() > last_switch_lsn)
 		{


[parent: 334b82cd56a6]