Re: AIO v2.5

On Tue, Mar 25, 2025 at 08:17:17PM -0400, Andres Freund wrote:
> On 2025-03-25 09:15:43 -0700, Noah Misch wrote:
> > On Tue, Mar 25, 2025 at 11:57:58AM -0400, Andres Freund wrote:
> > > FWIW, I prototyped this, it's not hard.
> > >
> > > But it can't replace the current WARNING with 100% fidelity: If we read 60
> > > blocks in a single smgrreadv, we today would would emit 60 WARNINGs. But we
> > > can't encode that many block offset in single PgAioResult, there's not enough
> > > space, and enlarging it far enough doesn't seem to make sense either.
> > >
> > >
> > > What we *could* do is to emit one WARNING for each bufmgr.c smgrstartreadv(),
> > > with that warning saying that there were N zeroed blocks in a read from block
> > > N to block Y and a HINT saying that there are more details in the server log.
>
> It should probably be DETAIL, not HINT...

Either is fine with me. I would go for HINT if referring to the server log,
given the precedent of errhint("See server log for query details."). DETAIL
fits for block counts, though:

> Could use some input on the framing of the message/detail. Right now it's:
>
> ERROR: invalid page in block 8 of relation base/5/16417
> DETAIL: Read of 8 blocks, starting at block 7, 1 other pages in the same read are invalid.
>
> But that doesn't seem great. Maybe:
>
> DETAIL: Read of blocks 7..14, 1 other pages in the same read were also invalid.
>
> But that still isn't really a sentence.

How about this for the multi-page case:

WARNING: zeroing out %u invalid pages among blocks %u..%u of relation %s
DETAIL: Block %u held first invalid page.
HINT: See server log for the other %u invalid blocks.

For the one-page case, the old message can stay:

WARNING: invalid page in block %u of relation %s; zeroing out page