Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>, Lukas Fittl <lukas(at)fittl(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: 2022-10-26 03:15:06
Message-ID: CAAKRu_a+bf=N+n-vcN1aKXDk3mPRXzrpYCZKBmhPEB6Pg8Su6w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

v35 is attached

On Mon, Oct 24, 2022 at 2:38 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> On Thu, Oct 20, 2022 at 1:31 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > I wonder if we should add a "source" output argument to
> > StrategyGetBuffer(). Then nearly all the counting can happen in
> > BufferAlloc().
>
> I think we can just check for BM_VALID being set before invalidating it
> in order to claim the buffer at the end of BufferAlloc(). Then we can
> count it as an eviction or reuse.

Done this in attached version

>
> > On 2022-10-19 15:26:51 -0400, Melanie Plageman wrote:
> > > I have made some major changes in this area to make the columns more
> > > useful. I have renamed and split "clocksweeps". It is now "evicted" and
> > > "freelist acquired". This makes it clear when a block must be evicted
> > > from a shared buffer must be and may help to identify misconfiguration
> > > of shared buffers.
> >
> > I'm not sure freelist acquired is really that useful? If we don't add it, we
> > should however definitely not count buffers from the freelist as evictions.
> >
> >
> > > There is some nuance here that I tried to make clear in the docs.
> > > "freelist acquired" in a shared context is straightforward.
> > > "freelist acquired" in a strategy context is counted when a shared
> > > buffer is added to the strategy ring (not when it is reused).
> >
> > Not sure what the second half here means - why would a buffer that's not from
> > the freelist ever be counted as being from the freelist?
> >
> >
> > > "freelist_acquired" is confusing for local buffers but I wanted to
> > > distinguish between reuse/eviction of local buffers and initial
> > > allocation. "freelist_acquired" seemed more fitting because there is a
> > > clocksweep to find a local buffer and if it hasn't been allocated yet it
> > > is allocated in a place similar to where shared buffers acquire a buffer
> > > from the freelist. If I didn't count it here, I would need to make a new
> > > column only for local buffers called "allocated" or something like that.
> >
> > I think you're making this too granular. We need to have more detail than
> > today. But we don't necessarily need to catch every nuance.

I cut freelist_acquired in attached version.

> I am fine with cutting freelist_acquired. The same actionable
> information that it could provide could be provided by "read", right?
> Also, removing it means I can remove the complicated explanation of how
> freelist_acquired should be interpreted in IOCONTEXT_LOCAL.
>
> Speaking of IOCONTEXT_LOCAL, I was wondering if it is confusing to call
> it IOCONTEXT_LOCAL since it refers to IO done for temporary tables. What
> if, in the future, we want to track other IO done using data in local
> memory? Also, what if we want to track other IO done using data from
> shared memory that is not in shared buffers? Would IOCONTEXT_SB and
> IOCONTEXT_TEMP be better? Should IOContext literally describe the
> context of the IO being done and there be a separate column which
> indicates the source of the data for the IO?
> Like wal_buffer, local_buffer, shared_buffer? Then if it is not
> block-oriented, it could be shared_mem, local_mem, or bypass?

pg_stat_statements uses local_blks_read and temp_blks_read for local
buffers for temp tables and temp file IO respectively -- so perhaps we
should stick to that

Other updates in this version:

I've also updated the unit column to bytes_conversion.

I've made quite a few updates to the docs including more information
on overlaps between pg_stat_database, pg_statio_*, and
pg_stat_statements.

Let me know if there are other configuration tip resources from the
existing docs that I could link in the column "files_synced".

I still need to look at the docs with fresh eyes and do another round of
cleanup (probably).

- Melanie

Attachment Content-Type Size
v35-0001-Remove-BufferAccessStrategyData-current_was_in_r.patch text/x-patch 3.8 KB
v35-0002-Track-IO-operation-statistics-locally.patch text/x-patch 30.5 KB
v35-0004-Add-system-view-tracking-IO-ops-per-backend-type.patch text/x-patch 47.4 KB
v35-0003-Aggregate-IO-operation-stats-per-BackendType.patch text/x-patch 22.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2022-10-26 03:19:48 Re: Allow file inclusion in pg_hba and pg_ident files
Previous Message David Rowley 2022-10-26 01:38:22 Re: Allow WindowFuncs prosupport function to use more optimal WindowClause options