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

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Magnus Hagander <magnus@hagander.net>, Pg Hackers <pgsql-hackers@postgresql.org>, Lukas Fittl <lukas@fittl.com>, Thomas Munro <thomas.munro@gmail.com>
Date: 2022-07-14T22:44:48Z
Lists: pgsql-hackers

Attachments

In addition to adding several new tests, the attached version 26 fixes a
major bug in constructing the view.

The only valid combination of IOPATH/IOOP that is not tested now is
IOPATH_STRATEGY + IOOP_WRITE. In most cases when I ran this in regress,
the checkpointer wrote out the dirty strategy buffer before VACUUM got
around to reusing and writing it out in my tests.

I've also changed the BACKEND_NUM_TYPES definition. Now arrays will have
that dead spot for B_INVALID, but I feel like it is much easier to
understand without trying to skip that spot and use those special helper
functions.

I also started skipping adding rows to the view for WAL_RECEIVER and
WAL_WRITER and for BackendTypes except B_BACKEND and WAL_SENDER for
IOPATH_LOCAL.

On Tue, Jul 12, 2022 at 1:18 PM Andres Freund <andres@anarazel.de> wrote:

> On 2022-07-11 22:22:28 -0400, Melanie Plageman wrote:
> > Yes, per an off list suggestion by you, I have changed the tests to use a
> > sum of writes. I've also added a test for IOPATH_LOCAL and fixed some of
> > the missing calls to count IO Operations for IOPATH_LOCAL and
> > IOPATH_STRATEGY.
> >
> > I struggled to come up with a way to test writes for a particular
> > type of backend are counted correctly since a dirty buffer could be
> > written out by another type of backend before the target BackendType has
> > a chance to write it out.
>
> I guess temp file writes would be reliably done by one backend... Don't
> have a
> good idea otherwise.
>
>
This was mainly an issue for IOPATH_STRATEGY writes as I mentioned. I
still have not solved this.


>
> > I'm not sure how to cause a strategy "extend" for testing.
>
> COPY into a table should work. But might be unattractive due to the size
> of of
> the COPY ringbuffer.
>

Did it with a CTAS as Horiguchi-san suggested.


>
> > > Would be nice to have something testing that the ringbuffer stats stuff
> > > does something sensible - that feels not entirely trivial.
> > >
> > >
> > I've added a test to test that reused strategy buffers are counted as
> > allocs. I would like to add a test which checks that if a buffer in the
> > ring is pinned and thus not reused, that it is not counted as a strategy
> > alloc, but I found it challenging without a way to pause vacuuming, pin
> > a buffer, then resume vacuuming.
>
> Yea, that's probably too hard to make reliable to be worth it.
>
>
Yes, I have skipped this.

- Melanie

Commits

  1. Stabilize pg_stat_io writes test

  2. Fix flakey pg_stat_io test

  3. Suppress more compiler warnings in new pgstats code.

  4. Suppress compiler warnings in new pgstats code.

  5. Add tests for pg_stat_io

  6. Create regress_tblspc in test_setup

  7. Add pg_stat_io view, providing more detailed IO statistics

  8. pgstat: Track more detailed relation IO statistics

  9. pgstat: Infrastructure for more detailed IO statistics

  10. doc: Fix some issues in logical replication section

  11. Manual cleanup and pgindent of pgstat and bufmgr related code

  12. Have the planner consider Incremental Sort for DISTINCT

  13. Use actual backend IDs in pg_stat_get_backend_idset() and friends.

  14. Remove redundant call to pgstat_report_wal()

  15. Add BackendType for standalone backends

  16. Initialize backend status reporting during bootstrap.