Hi, On 2025-03-19 13:20:17 -0400, Melanie Plageman wrote: > On Tue, Mar 18, 2025 at 4:12 PM Andres Freund <and...@anarazel.de> wrote: > > > > Attached is v2.10, > > I noticed a few comments could be improved in 0011: bufmgr: Use AIO > in StartReadBuffers() > [...]
Yep. > Above and in AsyncReadBuffers() > > * To support retries after short reads, the first operation->nblocks_done is > * buffers are skipped. > > can't quite understand this Heh, yea, it's easy to misunderstand. "short read" in the sense of a partial read, i.e. a preadv() that only read some of the blocks, not all. I'm replacing the "short" with partial. (also removed the superfluous "is") > * 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? I'd report it as additional time for the IO we're trying to start, as that wait would otherwise not happen. > And what is "accumulated" time here? It seems like you just add the time to > the running total and that is already accumulated. Afaict there currently is no way to report a time delta to pgstat. pgstat_count_io_op_time() computes the time since pgstat_prepare_io_time(). Due to the assertions that time cannot be reported for an operation with a zero count, we can't just do two pgstat_prepare_io_time(); ...; pgstat_count_io_op_time(); twice, with the first one passing cnt=0. Greetings, Andres Freund
