Re: Add pg_buffercache_evict_all() and pg_buffercache_mark_dirty[_all]() functions

From: Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Aidar Imamov <a(dot)imamov(at)postgrespro(dot)ru>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, koshy44(at)gmail(dot)com
Subject: Re: Add pg_buffercache_evict_all() and pg_buffercache_mark_dirty[_all]() functions
Date: 2025-04-11 08:02:28
Message-ID: CAN55FZ2Hi4uAeCcNAE1EH7DzxA0qwqt64SMJanwM0UKSqG6dCA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thank you for looking into this!

On Thu, 10 Apr 2025 at 23:06, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, Mar 18, 2025 at 6:03 PM Aidar Imamov <a(dot)imamov(at)postgrespro(dot)ru> wrote:
> > > for (int buf = 1; buf < NBuffers; buf++)
> > Mb it would be more correct to use <= NBuffers?
>
> I agree that (int buf = 1; buf < NBuffers; buf++) isn't right because
> that iterates one fewer times than the number of buffers. What was
> ultimately committed was:
>
> + for (int buf = 1; buf <= NBuffers; buf++)
> + {
> + BufferDesc *desc = GetBufferDescriptor(buf - 1);
>
> Curiously, there is no other instance of <= NBuffers in the code.
> Elsewhere we instead do:
>
> for (i = 0; i < NBuffers; i++)
> {
> BufferDesc *bufHdr = GetBufferDescriptor(i);
>
> Or in BufferSync:
>
> for (buf_id = 0; buf_id < NBuffers; buf_id++)
> {
> BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
>
> Nonetheless what was committed seems pretty defensible, because we
> have lots of other places that do GetBufferDescriptor(buffer - 1) and
> similar. Alternating between 0-based indexing and 1-based indexing
> like this seems rather error-prone somehow. :-(

I understand your point. I did it like that because bufferids start
from 1 and go to NBuffers inclusive in the pg_buffercache view, so it
seems more natural to me to implement it like that. I am okay to
replace these loops with [1] to make it standart everywhere:

[1]
for (int buf = 0; buf < NBuffers; buf++)
{
BufferDesc *desc = GetBufferDescriptor(buf);

--
Regards,
Nazir Bilal Yavuz
Microsoft

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2025-04-11 09:05:14 Re: Correct documentation for protocol version
Previous Message Nazir Bilal Yavuz 2025-04-11 08:01:21 Re: Add pg_buffercache_evict_all() and pg_buffercache_mark_dirty[_all]() functions