Re: pg_stat_io not tracking smgrwriteback() is confusing

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org, Amit Kapila <amit.kapila16@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2023-04-25T01:29:48Z
Lists: pgsql-hackers

Attachments

On Mon, Apr 24, 2023 at 7:02 PM Melanie Plageman <melanieplageman@gmail.com>
wrote:

> On Mon, Apr 24, 2023 at 03:56:54PM -0700, Andres Freund wrote:
> >
> > I was thinking we'd track writeback separately from the write, rather
> than
> > attributing the writeback to "write".  Otherwise it looks good, based on
> a
> > quick skim.
>
> Like you want a separate IOOp IOOP_WRITEBACK? Interesting. Okay.
>


Okay, attached v2 does this (adds IOOP_WRITEBACK).

With my patch applied and the same pgbench setup as you (for -T30):

After pgbench:

    backend_type     |    object     |  context  |  writes  | write_time |
writebacks | writeback_time |  fsyncs | fsync_time |
---------------------+---------------+-----------+----------+------------+------------+----------------+---------+------------+
 background writer   | relation      | normal    |     5581 |     23.416 |
      5568 |          32.33 |       0 |          0 |
 checkpointer        | relation      | normal    |    89116 |    295.576 |
     89106 |          416.5 |      84 |   5242.764 |


and then after a stats reset followed by an explicit checkpoint:


    backend_type     |    object     |  context  |  writes |     write_time
    | writebacks | writeback_time |  fsyncs | fsync_time |
---------------------+---------------+-----------+---------+--------------------+------------+----------------+---------+------------+
 checkpointer        | relation      | normal    |  229807 |
457.43600000000004 |     229817 |         532.84 |      52 |    378.652 |


I've yet to cook up a client backend test case (e.g. with COPY). I've taken
that as a todo.

I have a few outstanding questions:

1) Does it make sense for writebacks to count the number of blocks for
which writeback was requested or the number of calls to smgrwriteback() or
the number of actual syscalls made? We don't actually know from outside
of mdwriteback() how many FileWriteback() calls we will make.

2) I'm a little nervous about not including IOObject in the writeback
context. Technically, there is nothing stopping local buffer code from
calling IssuePendingWritebacks(). Right now, local buffer code doesn't
do ScheduleBufferTagForWriteback(). But it doesn't seem quite right to
hardcode in IOOBJECT_RELATION when there is nothing wrong with
requesting writeback of local buffers (AFAIK). What do you think?

3) Should any restrictions be added to pgstat_tracks_io_object() or
pgstat_tracks_io_op()? I couldn't think of any backend types or IO
contexts which would not do writeback as a rule. Also, though we don't
do writeback for temp tables now, it isn't nonsensical to do so. In
this version, I didn't add any restrictions.

Docs need work. I added a placeholder for the new columns. I'll update it
once we decide what writebacks should actually count. And, I don't think
we can do any kind of ongoing test.

- Melanie

Commits

  1. Add writeback to pg_stat_io

  2. Update parameter name context to wb_context

  3. Use BUFFER_USAGE_LIMIT to reduce needed test table size