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-24T23:02:47Z
Lists: pgsql-hackers
On Mon, Apr 24, 2023 at 03:56:54PM -0700, Andres Freund wrote: > Hi, > > On 2023-04-24 18:36:24 -0400, Melanie Plageman wrote: > > On Mon, Apr 24, 2023 at 6:13 PM Andres Freund <andres@anarazel.de> wrote: > > > > Also, it seems like this (given the current code) is only reachable for > > > > permanent relations (i.e. not for IO object temp relation). If other > > > backend > > > > types than checkpointer may call smgrwriteback(), we likely have to > > > consider > > > > the IO context. > > > > > > I think we should take it into account - it'd e.g. interesting to see a > > > COPY > > > is bottlenecked on smgrwriteback() rather than just writing the data. > > > > > > > With the quick and dirty attached patch and using your example but with a > > pgbench -T200 on my rather fast local NVMe SSD, you can still see quite > > a difference. > > Quite a difference between what? With and without the patch. Meaning: clearly tracking writeback is a good idea. > > What scale of pgbench did you use? 1000, as you did > > -T200 is likely not a good idea, because a timed checkpoint might "interfere", > unless you use a non-default checkpoint_timeout. A timed checkpoint won't show > the issue as easily, because checkpointer spend most of the time sleeping. Ah, I see. I did not use a non-default checkpoint timeout. > > Patch needs cleanup/comments and a bit more work, but I could do with > > a sanity check review on the approach. > > 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. - Melanie
Commits
-
Add writeback to pg_stat_io
- 093e5c57d506 16.0 landed
-
Update parameter name context to wb_context
- 52676dc2e016 16.0 landed
-
Use BUFFER_USAGE_LIMIT to reduce needed test table size
- 322875597c0c 16.0 landed