On 10/11/2025 13:41, Daniel Gustafsson wrote:
+ uint32 slru_pages_per_segment; /* size of each SLRU segment */
Should this be expanded ever so slightly? A new reader of the code might
wonder about the relationship between "pages_per" and "size".
Hmm, there's not much space for further explanations on that line. We
could add a longer multi-line comment but I'd rather keep it short and
consistent with the other similar fields around it. I hope that readers
who want more information will find the SLRU_PAGES_PER_SEGMENT
definition and the comments there.
I did consider renaming the field to 'slru_seg_size', to rhyme with
'relseg_size' and 'xlog_seg_size'. But then it wouldn't match the name
of SLRU_PAGES_PER_SEGMENT anymore. We could rename
SLRU_PAGES_PER_SEGMENT too, but I'm not sure it's worth the code churn,
and IMO "pages per segment" is better than "segment size" anyway because
it tells you what the unit is.
No objections (apart from the catversion =)) from reading the patch.
Thanks for the review!
- Heikki