Re: Using read stream in autoprewarm

On 03/04/2025 17:31, Melanie Plageman wrote:
> Attached v12 fixes a bug Bilal found off-list in 0002 related to
> handling invalid blocks.

I had a quick look at this. Looks good overall, some small remarks:

v12-0001-Autoprewarm-global-objects-separately.patch

> Instead, modify apw_load_buffers() to prewarm the shared objects in one
> invocation of autoprewarm_database_main() while connected to the first
> valid database.

So it effectively treats "global objects" as one extra database,
launching a separate worker process to handle global objects. It took me
a while to understand that. From the commit message, I understood that
it still does that within the first worker process invocation, but no. A
comment somewhere would be good.

One extra worker process invocation is obviously not an improvement
performance-wise, but seems acceptable.

v12-0002-Refactor-autoprewarm_database_main-in-preparatio.patch

Yes, I agree this makes the logic more clear

v12-0003-Use-streaming-read-I-O-in-autoprewarm.patch

I wonder if the have_free_buffer() calls work correctly with read
streams? Or will you "overshoot", prewarming a few more pages after the
buffer cache is already full? I guess that depends on when exactly the
read stream code allocates the buffer.

While reviewing this, I noticed a pre-existing bug: The code ignores
'tablespace' when deciding if it's reached the end of the current
relation. I believe it's possible to have two different relations with
the same relnumber, in different tablespaces.

--
Heikki Linnakangas
Neon (https://neon.tech)