Fix incorrect comment about the timing of AbsorbFsyncRequests() during

checkpoint.  The comment claimed that we could do this anytime after
setting the checkpoint REDO point, but actually BufferSync is relying
on the assumption that buffers dumped by other backends will be fsync'd
too.  So we really could not do it any sooner than we are doing it.

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index e0eb2a49ba3ffd9d42d355ee02aececeb74260a7..c919870a569811d70b4443b7d4d6a2ba57eb09fd 100644 (file)
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -1007,9 +1007,9 @@ ForwardFsyncRequest(RelFileNode rnode, BlockNumber segno)
  *             Retrieve queued fsync requests and pass them to local smgr.
  *
  * This is exported because it must be called during CreateCheckPoint;
- * we have to be sure we have accepted all pending requests *after* we
- * establish the checkpoint REDO pointer.  Since CreateCheckPoint
- * sometimes runs in non-bgwriter processes, do nothing if not bgwriter.
+ * we have to be sure we have accepted all pending requests just before
+ * we start fsync'ing.  Since CreateCheckPoint sometimes runs in
+ * non-bgwriter processes, do nothing if not bgwriter.
  */
 void
 AbsorbFsyncRequests(void)

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index a45fb2bd07fccebed4c70f34e4aa135ebefe22fd..99d771dd1083e6d83c89a4e2c537eb585573550e 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -880,9 +880,12 @@ mdsync(void)
 
        /*
         * If we are in the bgwriter, the sync had better include all fsync
-        * requests that were queued by backends before the checkpoint REDO
-        * point was determined.  We go that a little better by accepting all
-        * requests queued up to the point where we start fsync'ing.
+        * requests that were queued by backends up to this point.  The tightest
+        * race condition that could occur is that a buffer that must be written
+        * and fsync'd for the checkpoint could have been dumped by a backend
+        * just before it was visited by BufferSync().  We know the backend will
+        * have queued an fsync request before clearing the buffer's dirtybit,
+        * so we are safe as long as we do an Absorb after completing BufferSync().
         */
        AbsorbFsyncRequests();