localbuf: Introduce TerminateLocalBufferIO()

Previously TerminateLocalBufferIO() was open-coded in multiple places, which
doesn't seem like a great idea. While TerminateLocalBufferIO() currently is
rather simple, an upcoming patch requires additional code to be added to
TerminateLocalBufferIO(), making this modification particularly worthwhile.

For some reason FlushRelationBuffers() previously cleared BM_JUST_DIRTIED,
even though that's never set for temporary buffers. This is not carried over
as part of this change.

Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/CAAKRu_b9anbWzEs5AAF9WCvcEVmgz-1AkHSQ-CLLy-p7WHzvFw@mail.gmail.com

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 8243f4b244528e7a18f17a831681fee4883bf1f2..a716074467f8decd1cb7bdeda34aa185b15107dd 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1072,19 +1072,11 @@ ZeroAndLockBuffer(Buffer buffer, ReadBufferMode mode, bool already_valid)
        if (!isLocalBuf)
            LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_EXCLUSIVE);
 
+       /* Set BM_VALID, terminate IO, and wake up any waiters */
        if (isLocalBuf)
-       {
-           /* Only need to adjust flags */
-           uint32      buf_state = pg_atomic_read_u32(&bufHdr->state);
-
-           buf_state |= BM_VALID;
-           pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
-       }
+           TerminateLocalBufferIO(bufHdr, false, BM_VALID);
        else
-       {
-           /* Set BM_VALID, terminate IO, and wake up any waiters */
            TerminateBufferIO(bufHdr, false, BM_VALID, true);
-       }
    }
    else if (!isLocalBuf)
    {
@@ -1554,19 +1546,11 @@ WaitReadBuffers(ReadBuffersOperation *operation)
                                    relpath(operation->smgr->smgr_rlocator, forknum).str)));
            }
 
-           /* Terminate I/O and set BM_VALID. */
+           /* Set BM_VALID, terminate IO, and wake up any waiters */
            if (persistence == RELPERSISTENCE_TEMP)
-           {
-               uint32      buf_state = pg_atomic_read_u32(&bufHdr->state);
-
-               buf_state |= BM_VALID;
-               pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
-           }
+               TerminateLocalBufferIO(bufHdr, false, BM_VALID);
            else
-           {
-               /* Set BM_VALID, terminate IO, and wake up any waiters */
                TerminateBufferIO(bufHdr, false, BM_VALID, true);
-           }
 
            /* Report I/Os as completing individually. */
            TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, io_first_block + j,
@@ -4501,8 +4485,7 @@ FlushRelationBuffers(Relation rel)
                                        IOCONTEXT_NORMAL, IOOP_WRITE,
                                        io_start, 1, BLCKSZ);
 
-               buf_state &= ~(BM_DIRTY | BM_JUST_DIRTIED);
-               pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
+               TerminateLocalBufferIO(bufHdr, true, 0);
 
                pgBufferUsage.local_blks_written++;
 
@@ -5589,8 +5572,11 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits,
    buf_state = LockBufHdr(buf);
 
    Assert(buf_state & BM_IO_IN_PROGRESS);
+   buf_state &= ~BM_IO_IN_PROGRESS;
+
+   /* Clear earlier errors, if this IO failed, it'll be marked again */
+   buf_state &= ~BM_IO_ERROR;
 
-   buf_state &= ~(BM_IO_IN_PROGRESS | BM_IO_ERROR);
    if (clear_dirty && !(buf_state & BM_JUST_DIRTIED))
        buf_state &= ~(BM_DIRTY | BM_CHECKPOINT_NEEDED);
 

diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 5331091132d532cd3eb03ab13b9daf7450c9bdc2..86b1c4c7c680adcba4f8f66bd627ac83770af5ce 100644 (file)
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -235,7 +235,6 @@ GetLocalVictimBuffer(void)
     */
    if (pg_atomic_read_u32(&bufHdr->state) & BM_DIRTY)
    {
-       uint32      buf_state = pg_atomic_read_u32(&bufHdr->state);
        instr_time  io_start;
        SMgrRelation oreln;
        Page        localpage = (char *) LocalBufHdrGetBlock(bufHdr);
@@ -259,8 +258,7 @@ GetLocalVictimBuffer(void)
                                IOOP_WRITE, io_start, 1, BLCKSZ);
 
        /* Mark not-dirty now in case we error out below */
-       buf_state &= ~BM_DIRTY;
-       pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
+       TerminateLocalBufferIO(bufHdr, true, 0);
 
        pgBufferUsage.local_blks_written++;
    }
@@ -483,6 +481,31 @@ MarkLocalBufferDirty(Buffer buffer)
    pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
 }
 
+/*
+ * Like TerminateBufferIO, but for local buffers
+ */
+void
+TerminateLocalBufferIO(BufferDesc *bufHdr, bool clear_dirty, uint32 set_flag_bits)
+{
+   /* Only need to adjust flags */
+   uint32      buf_state = pg_atomic_read_u32(&bufHdr->state);
+
+   /* BM_IO_IN_PROGRESS isn't currently used for local buffers */
+
+   /* Clear earlier errors, if this IO failed, it'll be marked again */
+   buf_state &= ~BM_IO_ERROR;
+
+   if (clear_dirty)
+       buf_state &= ~BM_DIRTY;
+
+   buf_state |= set_flag_bits;
+   pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
+
+   /* local buffers don't track IO using resowners */
+
+   /* local buffers don't use the IO CV, as no other process can see buffer */
+}
+
 /*
  * InvalidateLocalBuffer -- mark a local buffer invalid.
  *

diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 8b32fb108b037e6a081e2a8567f563803896cdb4..4611a60d3e0ed7577243bc834e3ace2eab5192c6 100644 (file)
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -471,6 +471,8 @@ extern BlockNumber ExtendBufferedRelLocal(BufferManagerRelation bmr,
                                          Buffer *buffers,
                                          uint32 *extended_by);
 extern void MarkLocalBufferDirty(Buffer buffer);
+extern void TerminateLocalBufferIO(BufferDesc *bufHdr, bool clear_dirty,
+                                  uint32 set_flag_bits);
 extern void DropRelationLocalBuffers(RelFileLocator rlocator,
                                     ForkNumber forkNum,
                                     BlockNumber firstDelBlock);