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);