Re: BufferUsage counters' values have changed

From: Andres Freund <andres(at)anarazel(dot)de>
To: Karina Litskevich <litskevichkarina(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
Subject: Re: BufferUsage counters' values have changed
Date: 2023-09-13 18:59:39
Message-ID: 20230913185939.zt62s6p5o36yup3o@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2023-09-11 09:23:59 +0300, Karina Litskevich wrote:
> On Mon, Sep 11, 2023 at 9:08 AM Karina Litskevich <
> litskevichkarina(at)gmail(dot)com> wrote:
>
> > I found a bug that causes one of the differences. Wrong counter is
> > incremented
> > in ExtendBufferedRelLocal(). The attached patch fixes it and should be
> > applied
> > to REL_16_STABLE and master.
> >
>
> I've forgotten to attach the patch. Here it is.

> From 999a3d533a9b74c8568cc8a3d715c287de45dd2c Mon Sep 17 00:00:00 2001
> From: Karina Litskevich <litskevichkarina(at)gmail(dot)com>
> Date: Thu, 7 Sep 2023 17:44:40 +0300
> Subject: [PATCH v1] Fix local_blks_written counter incrementation
>
> ---
> src/backend/storage/buffer/localbuf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
> index 1735ec7141..567b8d15ef 100644
> --- a/src/backend/storage/buffer/localbuf.c
> +++ b/src/backend/storage/buffer/localbuf.c
> @@ -431,7 +431,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr,
>
> *extended_by = extend_by;
>
> - pgBufferUsage.temp_blks_written += extend_by;
> + pgBufferUsage.local_blks_written += extend_by;
>
> return first_block;
> }
> --
> 2.34.1
>

Ugh, you're right.

The naming of local vs temp here is pretty unfortunate imo. I wonder if we
ought to at least dd a comment to BufferUsage clarifying the situation? Just
reading the comments therein one would be hard pressed to figure out which of
the variables temp table activity should be added to.

I don't think we currently can write a test for this in the core tests, as the
relevant data isn't visible anywhere, iirc. Thus I added a test to
pg_stat_statements. Afaict it should be stable?

Running the attached patch through CI, planning to push after that succeeds,
unless somebody has a comment?

Greetings,

Andres Freund

Attachment Content-Type Size
v2-0001-Fix-tracking-of-temp-table-relation-extensions-as.patch text/x-diff 4.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-09-13 19:09:12 Re: [PATCH] Add native windows on arm64 support
Previous Message Nathan Bossart 2023-09-13 18:34:50 Re: Inefficiency in parallel pg_restore with many tables