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)