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)
