Re: pg_stat_io not tracking smgrwriteback() is confusing

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
Subject: Re: pg_stat_io not tracking smgrwriteback() is confusing
Date: 2023-04-25 01:29:48
Message-ID: CAAKRu_bqnU9Phy-Y3tKW8Oev5Y+Nqd2QXG6aHKHYHHWW1C6ihA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 24, 2023 at 7:02 PM Melanie Plageman <melanieplageman(at)gmail(dot)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

Attachment Content-Type Size
v2-0001-Add-writeback-to-pg_stat_io.patch text/x-patch 10.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-04-25 01:30:42 Re: buffer refcount leak in foreign batch insert code
Previous Message Justin Pryzby 2023-04-25 00:18:54 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables