aio, bufmgr: Comment fixes/improvements

Some of these comments have been wrong for a while (12f3867f5534), some I
recently introduced (da7226993fd, 55b454d0e14). This includes an update to a
comment in FlushBuffer(), which will be copied in a future commit.

These changes seem big enough to be worth doing in separate commits.

Suggested-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250319212530.80.nmisch@google.com

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a0c37532d2fa4f36e7fd96cbcfa9361aae39bcb8..c966c2e83afac9d9f8596d6d009d03d9984d159c 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4401,7 +4401,7 @@ maybe_adjust_io_workers(void)
            ++io_worker_count;
        }
        else
-           break;              /* XXX try again soon? */
+           break;              /* try again next time */
    }
 
    /* Too many running? */

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 5cac8cd738973da90b459fe3316fcd38bf47003e..03317b49025b4bd123d0918169169d9dcc917292 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3929,9 +3929,10 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
        XLogFlush(recptr);
 
    /*
-    * Now it's safe to write buffer to disk. Note that no one else should
-    * have been able to write it while we were busy with log flushing because
-    * only one process at a time can set the BM_IO_IN_PROGRESS bit.
+    * Now it's safe to write the buffer to disk. Note that no one else should
+    * have been able to write it, while we were busy with log flushing,
+    * because we got the exclusive right to perform I/O by setting the
+    * BM_IO_IN_PROGRESS bit.
     */
    bufBlock = BufHdrGetBlock(buf);
 
@@ -5499,9 +5500,6 @@ IsBufferCleanupOK(Buffer buffer)
 /*
  * Functions for buffer I/O handling
  *
- * Note: We assume that nested buffer I/O never occurs.
- * i.e at most one BM_IO_IN_PROGRESS bit is set per proc.
- *
  * Also note that these are used only for shared buffers, not local ones.
  */
 

diff --git a/src/include/storage/aio.h b/src/include/storage/aio.h
index 6b08304094330c222148caa7a769f336e9fd45f2..c6bf7c4ccf54886087b9088c1fe73797df935c74 100644 (file)
--- a/src/include/storage/aio.h
+++ b/src/include/storage/aio.h
@@ -80,7 +80,7 @@ typedef enum PgAioHandleFlags
 /*
  * The IO operations supported by the AIO subsystem.
  *
- * This could be in aio_internal.h, as it is not pubicly referenced, but
+ * This could be in aio_internal.h, as it is not publicly referenced, but
  * PgAioOpData currently *does* need to be public, therefore keeping this
  * public seems to make sense.
  */

diff --git a/src/include/storage/aio_internal.h b/src/include/storage/aio_internal.h
index fb0425ccbfcb4648e6d90bba3b50f065cdbd445b..7f18da2c85651638775a555f7bc56bf8123eccb1 100644 (file)
--- a/src/include/storage/aio_internal.h
+++ b/src/include/storage/aio_internal.h
@@ -34,6 +34,11 @@
  * linearly through all states.
  *
  * State changes should all go through pgaio_io_update_state().
+ *
+ * Note that the externally visible functions to start IO
+ * (e.g. FileStartReadV(), via pgaio_io_start_readv()) move an IO from
+ * PGAIO_HS_HANDED_OUT to at least PGAIO_HS_STAGED and at most
+ * PGAIO_HS_COMPLETED_LOCAL (at which point the handle will be reused).
  */
 typedef enum PgAioHandleState
 {
@@ -285,6 +290,9 @@ typedef struct IoMethodOps
    /*
     * Start executing passed in IOs.
     *
+    * Shall advance state to at least PGAIO_HS_SUBMITTED.  (By the time this
+    * returns, other backends might have advanced the state further.)
+    *
     * Will not be called if ->needs_synchronous_execution() returned true.
     *
     * num_staged_ios is <= PGAIO_SUBMIT_BATCH_SIZE.
@@ -293,12 +301,24 @@ typedef struct IoMethodOps
     */
    int         (*submit) (uint16 num_staged_ios, PgAioHandle **staged_ios);
 
-   /*
+   /* ---
     * Wait for the IO to complete. Optional.
     *
+    * On return, state shall be on of
+    * - PGAIO_HS_COMPLETED_IO
+    * - PGAIO_HS_COMPLETED_SHARED
+    * - PGAIO_HS_COMPLETED_LOCAL
+    *
+    * The callback must not block if the handle is already in one of those
+    * states, or has been reused (see pgaio_io_was_recycled()).  If, on
+    * return, the state is PGAIO_HS_COMPLETED_IO, state will reach
+    * PGAIO_HS_COMPLETED_SHARED without further intervention by the IO
+    * method.
+    *
     * If not provided, it needs to be guaranteed that the IO method calls
     * pgaio_io_process_completion() without further interaction by the
     * issuing backend.
+    * ---
     */
    void        (*wait_one) (PgAioHandle *ioh,
                             uint64 ref_generation);