Avoid WAL flush checks for unlogged buffers in GetVictimBuffer()

GetVictimBuffer() rejects a victim buffer if it is from a bulkread
strategy ring and reusing it would require flushing WAL. Unlogged table
buffers can have fake LSNs (e.g. unlogged GiST pages) and calling
XLogNeedsFlush() on a fake LSN is meaningless.

This is a bit of future-proofing because currently the bulkread strategy
is not used for relations with fake LSNs.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reported-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Andres Freund <andres@anarazel.de>
Earlier version reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/flat/fmkqmyeyy7bdpvcgkheb6yaqewemkik3ls6aaveyi5ibmvtxnd%40nu2kvy5rq3a6

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index ae15be6e38b54767bd15d2b7e71a1053d9b99aaf..8084858726fddad8ce887c093bd6a7040aa70205 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -2516,24 +2516,24 @@ again:
                }
 
                /*
-                * If using a nondefault strategy, and writing the buffer would
-                * require a WAL flush, let the strategy decide whether to go ahead
-                * and write/reuse the buffer or to choose another victim.  We need to
-                * hold the content lock in at least share-exclusive mode to safely
-                * inspect the page LSN, so this couldn't have been done inside
-                * StrategyGetBuffer.
+                * If using a nondefault strategy, and this victim came from the
+                * strategy ring, let the strategy decide whether to reject it when
+                * reusing it would require a WAL flush.  This only applies to
+                * permanent buffers; unlogged buffers can have fake LSNs, so
+                * XLogNeedsFlush() is not meaningful for them.
+                *
+                * We need to hold the content lock in at least share-exclusive mode
+                * to safely inspect the page LSN, so this couldn't have been done
+                * inside StrategyGetBuffer().
                 */
-               if (strategy != NULL)
+               if (strategy && from_ring &&
+                       buf_state & BM_PERMANENT &&
+                       XLogNeedsFlush(BufferGetLSN(buf_hdr)) &&
+                       StrategyRejectBuffer(strategy, buf_hdr, from_ring))
                {
-                       XLogRecPtr      lsn = BufferGetLSN(buf_hdr);
-
-                       if (XLogNeedsFlush(lsn)
-                               && StrategyRejectBuffer(strategy, buf_hdr, from_ring))
-                       {
-                               LockBuffer(buf, BUFFER_LOCK_UNLOCK);
-                               UnpinBuffer(buf_hdr);
-                               goto again;
-                       }
+                       LockBuffer(buf, BUFFER_LOCK_UNLOCK);
+                       UnpinBuffer(buf_hdr);
+                       goto again;
                }
 
                /* OK, do the I/O */