On Tue, Mar 18, 2025 at 4:12 PM Andres Freund <andres@anarazel.de> wrote:
>
> Attached is v2.10,
I noticed a few comments could be improved in 0011: bufmgr: Use AIO
in StartReadBuffers()
In WaitReadBuffers(), this comment is incomplete:
/*
- * Skip this block if someone else has already completed it. If an
- * I/O is already in progress in another backend, this will wait for
- * the outcome: either done, or something went wrong and we will
- * retry.
+ * If there is an IO associated with the operation, we may need to
+ * wait for it. It's possible for there to be no IO if
*/
In WaitReadBuffers(), too many thes
/*
* Most of the the the one IO we started will read in everything. But
* we need to deal with short reads and buffers not needing IO
* anymore.
*/
In ReadBuffersCanStartIO()
+ /*
+ * Unfortunately a false returned StartBufferIO() doesn't allow to
+ * distinguish between the buffer already being valid and IO already
+ * being in progress. Since IO already being in progress is quite
+ * rare, this approach seems fine.
+ */
maybe reword "a false returned StartBufferIO()"
Above and in AsyncReadBuffers()
* To support retries after short reads, the first operation->nblocks_done is
* buffers are skipped.
can't quite understand this
+ * On return *nblocks_progres is updated to reflect the number of buffers
progress spelled wrong
* A secondary benefit is that this would allows us to measure the time in
* pgaio_io_acquire() without causing undue timer overhead in the common,
* non-blocking, case. However, currently the pgstats infrastructure
* doesn't really allow that, as it a) asserts that an operation can't
* have time without operations b) doesn't have an API to report
* "accumulated" time.
*/
allows->allow
What would the time spent in pgaio_io_acquire() be reported as? Time
submitting IOs? Time waiting for a handle? And what is "accumulated"
time here? It seems like you just add the time to the running total
and that is already accumulated.
- Melanie
