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: 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>, Lukas Fittl <lukas(at)fittl(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: 2022-07-05 17:24:55
Message-ID: CAAKRu_Y_B9pLm1-b6A5CfRhuWEMMy2iMy1zNG0_3GhejD-ZsMg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 21, 2022 at 8:15 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> On 2022-02-19 11:06:18 -0500, Melanie Plageman wrote:
> > v21 rebased with compile errors fixed is attached.
>
> This currently doesn't apply (mea culpa likely):
> http://cfbot.cputube.org/patch_37_3272.log
>
> Could you rebase? Marked as waiting-on-author for now.
>
>
>
Attached is the rebased/rewritten version of the pg_stat_buffers patch
which uses the cumulative stats system instead of stats collector.

I've moved to the model of backend-local pending stats which get
accumulated into shared memory by pgstat_report_stat().

It is worth noting that, with this method, other backends will no longer
have access to each other's individual IO operation statistics. An
argument could be made to keep the statistics in each backend in
PgBackendStatus before accumulating them to the cumulative stats system
so that they can be accessed at the per-backend level of detail.

There are two TODOs related to when pgstat_report_io_ops() should be
called. pgstat_report_io_ops() is meant for backends that will not
commonly call pgstat_report_stat(). I was unsure if it made sense for
BootstrapModeMain() to explicitly call pgstat_report_io_ops() and if
auto vacuum worker should call it explicitly and, if so, if it was the
right location to call it after do_autovacuum().

Archiver and syslogger do not increment or report IO operations.

I did not change pg_stat_bgwriter fields to derive from the IO
operations statistics structures since the reset targets differ.

Also, I added one test, but I'm not sure if it will be flakey. It tests
that the "writes" for checkpointer are tracked when data is inserted
into a table and then CHECKPOINT is explicitly invoked directly after. I
don't know if this will have a problem if the checkpointer is busy and
somehow the backend which dirtied the buffer is forced to write out its
own buffer, causing the test to potentially fail (even if the
checkpointer is doing other writes [causing it to be busy], it may not
do them in between the INSERT and the SELECT from pg_stat_buffers).

I am wondering how to add a non-flakey test. For regular backends, I
couldn't think of a way to suspend checkpointer to make them do their
own writes and fsyncs in the context of a regression or isolation test.
In fact for many of the dirty buffers it seems like it will be difficult
to keep bgwriter, checkpointer, and regular backends from competing and
sometimes causing test failures.

- Melanie

Attachment Content-Type Size
v22-0002-Track-IO-operation-statistics.patch application/octet-stream 32.6 KB
v22-0003-Add-system-view-tracking-IO-ops-per-backend-type.patch application/octet-stream 13.6 KB
v22-0001-Add-BackendType-for-standalone-backends.patch application/octet-stream 2.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-07-05 17:38:43 Re: pg_checkpointer is not a verb or verb phrase
Previous Message Tom Lane 2022-07-05 17:07:48 Re: pg_upgrade (12->14) fails on aggregate