> On 17 Nov 2023, at 16:11, Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Fri, Nov 17, 2023 at 1:09 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>>
>> On Thu, Nov 16, 2023 at 3:11 PM Alvaro Herrera <alvhe...@alvh.no-ip.org>
>> wrote:
>
> PFA, updated patch version, this fixes the comment given by Alvaro and
> also improves some of the comments.
I’ve skimmed through the patch set. Here are some minor notes.
1. Cycles “for (slotno = bankstart; slotno < bankend; slotno++)” in
SlruSelectLRUPage() and SimpleLruReadPage_ReadOnly() now have identical
comments. I think a little of copy-paste is OK.
But SimpleLruReadPage_ReadOnly() does pgstat_count_slru_page_hit(), while
SlruSelectLRUPage() does not. This is not related to the patch set, just a code
nearby.
2. Do we really want these functions doing all the same?
extern bool check_multixact_offsets_buffers(int *newval, void **extra,GucSource
source);
extern bool check_multixact_members_buffers(int *newval, void **extra,GucSource
source);
extern bool check_subtrans_buffers(int *newval, void **extra,GucSource source);
extern bool check_notify_buffers(int *newval, void **extra, GucSource source);
extern bool check_serial_buffers(int *newval, void **extra, GucSource source);
extern bool check_xact_buffers(int *newval, void **extra, GucSource source);
extern bool check_commit_ts_buffers(int *newval, void **extra,GucSource source);
3. The name SimpleLruGetSLRUBankLock() contains meaning of SLRU twice. I’d
suggest truncating prefix of infix.
I do not have hard opinion on any of this items.
Best regards, Andrey Borodin.
