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
