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

2023-04-25 Thread Pavel Luzanov
On 24.04.2023 23:53, Melanie Plageman wrote: I copied the committer who most recently touched pg_stat_io (Michael Paquier) to see if we could get someone interested in committing this docs update. I can explain my motivation by suggesting this update. pg_stat_io is a very impressive feature.

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

2023-04-24 Thread Melanie Plageman
On Mon, Apr 10, 2023 at 3:41 AM Pavel Luzanov wrote: > > On 05.04.2023 03:41, Melanie Plageman wrote: > > On Tue, Apr 4, 2023 at 4:35 PM Pavel Luzanov > > wrote: > > > >> After a little thought... I'm not sure about the term 'bootstrap > >> process'. I can't find this term in the documentation.

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

2023-04-10 Thread Pavel Luzanov
On 05.04.2023 03:41, Melanie Plageman wrote: On Tue, Apr 4, 2023 at 4:35 PM Pavel Luzanov wrote: After a little thought... I'm not sure about the term 'bootstrap process'. I can't find this term in the documentation. There are various mentions of "bootstrap" peppered throughout the docs but

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

2023-04-04 Thread Melanie Plageman
On Tue, Apr 4, 2023 at 4:35 PM Pavel Luzanov wrote: > > On 03.04.2023 23:50, Melanie Plageman wrote: > > Attached is a tiny patch to add standalone backend type to > > pg_stat_activity documentation (referenced by pg_stat_io). > > > > I mentioned both the bootstrap process and single user mode

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

2023-04-04 Thread Pavel Luzanov
On 03.04.2023 23:50, Melanie Plageman wrote: Attached is a tiny patch to add standalone backend type to pg_stat_activity documentation (referenced by pg_stat_io). I mentioned both the bootstrap process and single user mode process in the docs, though I can't imagine that the bootstrap process

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

2023-04-03 Thread Melanie Plageman
On Mon, Apr 3, 2023 at 12:13 AM Pavel Luzanov wrote: > > Hello, > > I found that the 'standalone backend' backend type is not documented > right now. > Adding something like (from commit message) would be helpful: > > Both the bootstrap backend and single user mode backends will have >

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

2023-04-02 Thread Pavel Luzanov
Hello, I found that the 'standalone backend' backend type is not documented right now. Adding something like (from commit message) would be helpful: Both the bootstrap backend and single user mode backends will have backend_type STANDALONE_BACKEND. -- Pavel Luzanov Postgres Professional:

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

2023-03-10 Thread Melanie Plageman
On Fri, Mar 10, 2023 at 3:19 PM Justin Pryzby wrote: > > On Thu, Mar 09, 2023 at 11:43:01AM -0800, Andres Freund wrote: > > > https://api.cirrus-ci.com/v1/artifact/task/6701069548388352/log/src/test/recovery/tmp_check/regression.diffs > > > > Hm. I guess the explanation here is that the buffers

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

2023-03-10 Thread Justin Pryzby
On Thu, Mar 09, 2023 at 11:43:01AM -0800, Andres Freund wrote: > On 2023-03-09 06:51:31 -0600, Justin Pryzby wrote: > > On Tue, Mar 07, 2023 at 10:18:44AM -0800, Andres Freund wrote: > > > Hi, > > > > > > On 2023-03-06 15:21:14 -0500, Melanie Plageman wrote: > > > > Good point. Attached is what

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

2023-03-10 Thread Melanie Plageman
On Thu, Mar 9, 2023 at 2:43 PM Andres Freund wrote: > On 2023-03-09 06:51:31 -0600, Justin Pryzby wrote: > > On Tue, Mar 07, 2023 at 10:18:44AM -0800, Andres Freund wrote: > > There's a 2nd portion of the test that's still flapping, at least on > > cirrusci. > > > > The issue that Tom mentioned

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

2023-03-09 Thread Andres Freund
Hi, On 2023-03-09 06:51:31 -0600, Justin Pryzby wrote: > On Tue, Mar 07, 2023 at 10:18:44AM -0800, Andres Freund wrote: > > Hi, > > > > On 2023-03-06 15:21:14 -0500, Melanie Plageman wrote: > > > Good point. Attached is what you suggested. I committed the transaction > > > before the drop table

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

2023-03-09 Thread Justin Pryzby
On Tue, Mar 07, 2023 at 10:18:44AM -0800, Andres Freund wrote: > Hi, > > On 2023-03-06 15:21:14 -0500, Melanie Plageman wrote: > > Good point. Attached is what you suggested. I committed the transaction > > before the drop table so that the statistics would be visible when we > > queried

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

2023-03-07 Thread Andres Freund
Hi, On 2023-03-06 15:21:14 -0500, Melanie Plageman wrote: > Good point. Attached is what you suggested. I committed the transaction > before the drop table so that the statistics would be visible when we > queried pg_stat_io. Pushed, thanks for the report, analysis and fix, Tom, Horiguchi-san,

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

2023-03-06 Thread Kyotaro Horiguchi
At Mon, 6 Mar 2023 15:21:14 -0500, Melanie Plageman wrote in > On Mon, Mar 6, 2023 at 2:34 PM Andres Freund wrote: > > That, but also because it's simply more reliable. autovacuum=off doesn't > > protect against a anti-wraparound vacuum or such. Or a concurrent test > > somehow > > triggering

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

2023-03-06 Thread Melanie Plageman
On Mon, Mar 6, 2023 at 2:34 PM Andres Freund wrote: > > Hi, > > On 2023-03-06 14:24:09 -0500, Melanie Plageman wrote: > > On Mon, Mar 06, 2023 at 11:09:19AM -0800, Andres Freund wrote: > > > On 2023-03-06 10:09:24 -0500, Melanie Plageman wrote: > > > > On Mon, Mar 6, 2023 at 1:48 AM Kyotaro

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

2023-03-06 Thread Andres Freund
Hi, On 2023-03-06 14:24:09 -0500, Melanie Plageman wrote: > On Mon, Mar 06, 2023 at 11:09:19AM -0800, Andres Freund wrote: > > On 2023-03-06 10:09:24 -0500, Melanie Plageman wrote: > > > On Mon, Mar 6, 2023 at 1:48 AM Kyotaro Horiguchi > > > wrote: > > > > > > > > At Mon, 06 Mar 2023 15:24:25

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

2023-03-06 Thread Melanie Plageman
On Mon, Mar 06, 2023 at 11:09:19AM -0800, Andres Freund wrote: > Hi, > > On 2023-03-06 10:09:24 -0500, Melanie Plageman wrote: > > On Mon, Mar 6, 2023 at 1:48 AM Kyotaro Horiguchi > > wrote: > > > > > > At Mon, 06 Mar 2023 15:24:25 +0900 (JST), Kyotaro Horiguchi > > > wrote in > > > > In any

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

2023-03-06 Thread Andres Freund
Hi, On 2023-03-06 10:09:24 -0500, Melanie Plageman wrote: > On Mon, Mar 6, 2023 at 1:48 AM Kyotaro Horiguchi > wrote: > > > > At Mon, 06 Mar 2023 15:24:25 +0900 (JST), Kyotaro Horiguchi > > wrote in > > > In any case, I think we need to avoid such concurrent autovacuum/analyze. > > > > If it

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

2023-03-06 Thread Melanie Plageman
On Mon, Mar 6, 2023 at 1:48 AM Kyotaro Horiguchi wrote: > > At Mon, 06 Mar 2023 15:24:25 +0900 (JST), Kyotaro Horiguchi > wrote in > > In any case, I think we need to avoid such concurrent autovacuum/analyze. > > If it is correct, I believe the attached fix works. Thanks for investigating

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

2023-03-05 Thread Kyotaro Horiguchi
At Mon, 06 Mar 2023 15:24:25 +0900 (JST), Kyotaro Horiguchi wrote in > In any case, I think we need to avoid such concurrent autovacuum/analyze. If it is correct, I believe the attached fix works. regads. -- Kyotaro Horiguchi NTT Open Source Software Center diff --git

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

2023-03-05 Thread Kyotaro Horiguchi
At Sat, 04 Mar 2023 18:21:09 -0500, Tom Lane wrote in > Andres Freund writes: > > Just pushed the actual pg_stat_io view, the splitting of the tablespace > > test, > > and the pg_stat_io tests. > > One of the test cases is flapping a bit: > > diff -U3 >

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

2023-03-04 Thread Tom Lane
Andres Freund writes: > Just pushed the actual pg_stat_io view, the splitting of the tablespace test, > and the pg_stat_io tests. One of the test cases is flapping a bit: diff -U3 /home/pg/build-farm-15/buildroot/HEAD/pgsql.build/src/test/regress/expected/stats.out

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

2023-02-27 Thread Andres Freund
On 2023-02-27 14:58:30 -0500, Tom Lane wrote: > Agreed. I'll push this along with the earlier patch if there are > not objections. None here.

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

2023-02-27 Thread Tom Lane
Melanie Plageman writes: > On Mon, Feb 27, 2023 at 10:30 AM Tom Lane wrote: >> The risk of needing to cast when using the "int" loop variable >> as an enum is obviously the downside of that approach, but we have >> not seen any indication that any compilers actually do warn. >> It's interesting

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

2023-02-27 Thread Melanie Plageman
On Mon, Feb 27, 2023 at 10:30 AM Tom Lane wrote: > > Melanie Plageman writes: > > Attached is a patch to remove the *_FIRST macros. > > I was going to add in code to change > > > for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; > > io_object++) > > to > > for (IOObject

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

2023-02-27 Thread Tom Lane
Melanie Plageman writes: > Attached is a patch to remove the *_FIRST macros. > I was going to add in code to change > for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++) > to > for (IOObject io_object = 0; (int) io_object < IOOBJECT_NUM_TYPES; > io_object++) I

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

2023-02-27 Thread Melanie Plageman
On Sun, Feb 26, 2023 at 1:52 PM Tom Lane wrote: > > I wrote: > > The issue seems to be that code like this: > > ... > > is far too cute for its own good. > > Oh, there's another thing here that qualifies as too-cute: loops like > > for (IOObject io_object = IOOBJECT_FIRST; >

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

2023-02-26 Thread Melanie Plageman
On Sun, Feb 26, 2023 at 04:11:45PM -0500, Melanie Plageman wrote: > On Sun, Feb 26, 2023 at 12:33:03PM -0800, Andres Freund wrote: > > On 2023-02-26 15:08:33 -0500, Tom Lane wrote: > > > Andres Freund writes: > > > > They're all animals for testing older LLVM versions. They're using > > > >

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

2023-02-26 Thread Melanie Plageman
On Sun, Feb 26, 2023 at 12:33:03PM -0800, Andres Freund wrote: > On 2023-02-26 15:08:33 -0500, Tom Lane wrote: > > Andres Freund writes: > > > They're all animals for testing older LLVM versions. They're using > > > pretty old clang versions. phycodurus and dragonet are clang 3.9, > > > petalura

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

2023-02-26 Thread Andres Freund
On 2023-02-26 15:08:33 -0500, Tom Lane wrote: > Andres Freund writes: > > They're all animals for testing older LLVM versions. They're using > > pretty old clang versions. phycodurus and dragonet are clang 3.9, petalura > > and > > desmoxytes is clang 4, idiacanthus and pogona are clang 5. > >

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

2023-02-26 Thread Tom Lane
Andres Freund writes: > They're all animals for testing older LLVM versions. They're using > pretty old clang versions. phycodurus and dragonet are clang 3.9, petalura and > desmoxytes is clang 4, idiacanthus and pogona are clang 5. [ shrug ... ] If I thought this was actually good code, I

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

2023-02-26 Thread Andres Freund
Hi, On 2023-02-26 14:40:00 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2023-02-26 13:20:00 -0500, Tom Lane wrote: > >> I hadn't run my buildfarm-compile-warning scraper for a little while, > >> but I just did, and I find that this commit is causing warnings on > >> no fewer than 14

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

2023-02-26 Thread Tom Lane
Andres Freund writes: > On 2023-02-26 13:20:00 -0500, Tom Lane wrote: >> I hadn't run my buildfarm-compile-warning scraper for a little while, >> but I just did, and I find that this commit is causing warnings on >> no fewer than 14 buildfarm animals. They all look like > What other animals? If

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

2023-02-26 Thread Andres Freund
Hi, On 2023-02-26 13:20:00 -0500, Tom Lane wrote: > Andres Freund writes: > > Pushed the first (and biggest) commit. More tomorrow. > > I hadn't run my buildfarm-compile-warning scraper for a little while, > but I just did, and I find that this commit is causing warnings on > no fewer than 14

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

2023-02-26 Thread Tom Lane
I wrote: > The issue seems to be that code like this: > ... > is far too cute for its own good. Oh, there's another thing here that qualifies as too-cute: loops like for (IOObject io_object = IOOBJECT_FIRST; io_object < IOOBJECT_NUM_TYPES; io_object++) make it look like we could

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

2023-02-26 Thread Tom Lane
Andres Freund writes: > Pushed the first (and biggest) commit. More tomorrow. I hadn't run my buildfarm-compile-warning scraper for a little while, but I just did, and I find that this commit is causing warnings on no fewer than 14 buildfarm animals. They all look like ayu |

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

2023-02-21 Thread Justin Pryzby
On Tue, Feb 21, 2023 at 07:50:35PM -0600, Justin Pryzby wrote: > On Sat, Feb 11, 2023 at 10:24:37AM -0800, Andres Freund wrote: > > On 2023-02-08 21:03:19 -0800, Andres Freund wrote: > > > Pushed the first (and biggest) commit. More tomorrow. > > > > Just pushed the actual pg_stat_io view, the

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

2023-02-21 Thread Justin Pryzby
On Sat, Feb 11, 2023 at 10:24:37AM -0800, Andres Freund wrote: > On 2023-02-08 21:03:19 -0800, Andres Freund wrote: > > Pushed the first (and biggest) commit. More tomorrow. > > Just pushed the actual pg_stat_io view, the splitting of the tablespace test, > and the pg_stat_io tests. pg_stat_io

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

2023-02-14 Thread Kyotaro Horiguchi
At Tue, 14 Feb 2023 22:35:01 -0800, Maciek Sakrejda wrote in > On Tue, Feb 14, 2023 at 11:08 AM Andres Freund wrote: > > One thing I started to wonder about since is whether we should remove the > > io_ > > prefix from io_object, io_context. The prefixes make sense on the C level, > > but >

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

2023-02-14 Thread Maciek Sakrejda
On Tue, Feb 14, 2023 at 11:08 AM Andres Freund wrote: > One thing I started to wonder about since is whether we should remove the io_ > prefix from io_object, io_context. The prefixes make sense on the C level, but > it's not clear to me that that's also the case on the table level. Yeah, +1.

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

2023-02-14 Thread Andres Freund
Hi, On 2023-02-11 10:24:37 -0800, Andres Freund wrote: > Just pushed the actual pg_stat_io view, the splitting of the tablespace test, > and the pg_stat_io tests. One thing I started to wonder about since is whether we should remove the io_ prefix from io_object, io_context. The prefixes make

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

2023-02-11 Thread Andres Freund
Hi, On 2023-02-08 21:03:19 -0800, Andres Freund wrote: > Pushed the first (and biggest) commit. More tomorrow. Just pushed the actual pg_stat_io view, the splitting of the tablespace test, and the pg_stat_io tests. Yay! Thanks all for patch and review! > Already can't wait to see incremental

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

2023-02-08 Thread Andres Freund
Hi, On 2023-02-07 22:38:14 -0800, Andres Freund wrote: > I did another read through the series. I do have some minor changes, but > they're minor. I think this is ready for commit. I plan to start pushing > tomorrow. Pushed the first (and biggest) commit. More tomorrow. Already can't wait to

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

2023-02-07 Thread Kyotaro Horiguchi
At Tue, 7 Feb 2023 22:38:14 -0800, Andres Freund wrote in > Hi, > > I did another read through the series. I do have some minor changes, but > they're minor. I think this is ready for commit. I plan to start pushing > tomorrow. > > The changes I made are: > - the tablespace test changes didn't

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

2023-02-07 Thread Andres Freund
Hi, I did another read through the series. I do have some minor changes, but they're minor. I think this is ready for commit. I plan to start pushing tomorrow. The changes I made are: - the tablespace test changes didn't quite work in isolation / needed a bit of polishing - moved the

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

2023-01-24 Thread Kyotaro Horiguchi
At Tue, 24 Jan 2023 14:35:12 -0800, Andres Freund wrote in > > 0002: > > > > + Assert(pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)); > > > > This is relatively complex checking. We already asserts-out increments > > of invalid counters. Thus this is checking if some unrelated

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

2023-01-24 Thread Andres Freund
Hi, On 2023-01-24 17:22:03 +0900, Kyotaro Horiguchi wrote: > Hello. > > At Thu, 19 Jan 2023 21:15:34 -0500, Melanie Plageman > wrote in > > Oh dear-- an extra FlushBuffer() snuck in there somehow. > > Removed it in attached v51. > > Also, I fixed an issue in my tablespace.sql updates > > I

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

2023-01-24 Thread Kyotaro Horiguchi
At Tue, 24 Jan 2023 17:22:03 +0900 (JST), Kyotaro Horiguchi wrote in > +pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op) > +{ > + Assert(io_object < IOOBJECT_NUM_TYPES); > + Assert(io_context < IOCONTEXT_NUM_TYPES); > + Assert(io_op < IOOP_NUM_TYPES); > +

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

2023-01-24 Thread Kyotaro Horiguchi
Hello. At Thu, 19 Jan 2023 21:15:34 -0500, Melanie Plageman wrote in > Oh dear-- an extra FlushBuffer() snuck in there somehow. > Removed it in attached v51. > Also, I fixed an issue in my tablespace.sql updates I only looked 0002 and 0004. (Sorry for the random order of the comment..) 0002:

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

2023-01-19 Thread vignesh C
On Wed, 18 Jan 2023 at 03:30, Melanie Plageman wrote: > > v49 attached > > On Tue, Jan 17, 2023 at 2:12 PM Andres Freund wrote: > > On 2023-01-17 12:22:14 -0500, Melanie Plageman wrote: > > > > > > > +typedef struct PgStat_BackendIO > > > > > +{ > > > > > + PgStat_Counter > > > > >

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

2023-01-17 Thread Maciek Sakrejda
On Tue, Jan 17, 2023 at 9:22 AM Melanie Plageman wrote: > On Mon, Jan 16, 2023 at 4:42 PM Maciek Sakrejda wrote: > > I missed a couple of versions, but I think the docs are clearer now. > > I'm torn on losing some of the detail, but overall I do think it's a > > good trade-off. Moving some

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

2023-01-17 Thread Melanie Plageman
v49 attached On Tue, Jan 17, 2023 at 2:12 PM Andres Freund wrote: > On 2023-01-17 12:22:14 -0500, Melanie Plageman wrote: > > > > > +typedef struct PgStat_BackendIO > > > > +{ > > > > + PgStat_Counter > > > > data[IOCONTEXT_NUM_TYPES][IOOBJECT_NUM_TYPES][IOOP_NUM_TYPES]; > > > > +}

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

2023-01-17 Thread Andres Freund
Hi, On 2023-01-17 12:22:14 -0500, Melanie Plageman wrote: > > > @@ -359,6 +360,15 @@ static const PgStat_KindInfo > > > pgstat_kind_infos[PGSTAT_NUM_KINDS] = { > > > .snapshot_cb = pgstat_checkpointer_snapshot_cb, > > > }, > > > > > > + [PGSTAT_KIND_IO] = { > > > +

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

2023-01-16 Thread Maciek Sakrejda
On Fri, Jan 13, 2023 at 10:38 AM Melanie Plageman wrote: > > Attached is v47. I missed a couple of versions, but I think the docs are clearer now. I'm torn on losing some of the detail, but overall I do think it's a good trade-off. Moving some details out to after the table does keep the bulk of

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

2023-01-13 Thread Andres Freund
Hi, On 2023-01-13 13:38:15 -0500, Melanie Plageman wrote: > > I think that's marginally better, but I think having to define both > > FIRST and NUM is excessive and doesn't make it less fragile. Not sure > > what anyone else will say, but I'd prefer if it started at "0". The reason for using

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

2023-01-12 Thread Justin Pryzby
On Thu, Jan 12, 2023 at 09:19:36PM -0500, Melanie Plageman wrote: > On Wed, Jan 11, 2023 at 4:58 PM Justin Pryzby wrote: > > > > > Subject: [PATCH v45 4/5] Add system view tracking IO ops per backend type > > > > The patch can/will fail with: > > > > CREATE TABLESPACE test_io_shared_stats_tblspc

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

2023-01-11 Thread Justin Pryzby
> Subject: [PATCH v45 4/5] Add system view tracking IO ops per backend type The patch can/will fail with: CREATE TABLESPACE test_io_shared_stats_tblspc LOCATION ''; +WARNING: tablespaces created by regression test cases should have names starting with "regress_" CREATE TABLESPACE test_stats

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

2023-01-11 Thread vignesh C
On Tue, 10 Jan 2023 at 02:41, Melanie Plageman wrote: > > Attached is v45 of the patchset. I've done some additional code cleanup > and changes. The most significant change, however, is the docs. I've > separated the docs into its own patch for ease of review. > > The docs patch here was edited

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

2023-01-09 Thread Melanie Plageman
Attached is v45 of the patchset. I've done some additional code cleanup and changes. The most significant change, however, is the docs. I've separated the docs into its own patch for ease of review. The docs patch here was edited and co-authored by Samay Sharma. I'm not sure if the order of

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

2022-12-28 Thread Andres Freund
Hi, On 2022-10-06 13:42:09 -0400, Melanie Plageman wrote: > > Additionally, some minor notes: > > > > - Since the stats are counting blocks, it would make sense to prefix the > > view columns with "blks_", and word them in the past tense (to match > > current style), i.e. "blks_written",

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

2022-12-07 Thread Maciek Sakrejda
In the pg_stat_statements docs, there are several column descriptions like Total number of ... by the statement You added an additional sentence to some describing the equivalent pg_stat_io values, but you only added a period to the previous sentence for shared_blks_read (for other columns,

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

2022-12-05 Thread Andres Freund
Hi, - I think it might be worth to rename IOCONTEXT_BUFFER_POOL to IOCONTEXT_{NORMAL, PLAIN, DEFAULT}. I'd like at some point to track WAL IO , temporary file IO etc, and it doesn't seem useful to define a version of BUFFER_POOL for each of them. And it'd make it less confusing, because all

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

2022-12-04 Thread Maciek Sakrejda
On Tue, Nov 29, 2022 at 5:13 PM Melanie Plageman wrote: > Thanks for the review, Maciek! > > I've attached a new version 39 of the patch which addresses your docs > feedback from this email as well as docs feedback from Andres in [1] and > Justin in [2]. This looks great! Just a couple of minor

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

2022-11-29 Thread Justin Pryzby
On Mon, Nov 28, 2022 at 09:08:36PM -0500, Melanie Plageman wrote: > > +pgstat_io_op_stats_collected(BackendType bktype) > > +{ > > + return bktype != B_INVALID && bktype != B_ARCHIVER && bktype != > > B_LOGGER && > > + bktype != B_WAL_RECEIVER && bktype != B_WAL_WRITER; > > >

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

2022-11-28 Thread Melanie Plageman
On Wed, Nov 23, 2022 at 12:43 AM Justin Pryzby wrote: > > Note that 001 fails to compile without 002: > > ../src/backend/storage/buffer/bufmgr.c:1257:43: error: ‘from_ring’ undeclared > (first use in this function) > 1257 | StrategyRejectBuffer(strategy, buf, from_ring)) Thanks! I fixed

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

2022-11-25 Thread Andres Freund
Hi, On 2022-11-22 23:43:29 -0600, Justin Pryzby wrote: > I think there may be a problem/deficiency with hint bits: > > |postgres=# DROP TABLE u2; CREATE TABLE u2 AS SELECT > generate_series(1,99)a; SELECT pg_stat_reset_shared('io'); explain > (analyze,buffers) SELECT * FROM u2; > |... > |

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

2022-11-22 Thread Justin Pryzby
Note that 001 fails to compile without 002: ../src/backend/storage/buffer/bufmgr.c:1257:43: error: ‘from_ring’ undeclared (first use in this function) 1257 | StrategyRejectBuffer(strategy, buf, from_ring)) My "warnings" script informed me about these gripes from MSVC: [03:42:30.607]

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

2022-11-20 Thread Andres Freund
Hi, One good follow up patch will be to rip out the accounting for pg_stat_bgwriter's buffers_backend, buffers_backend_fsync and perhaps buffers_alloc and replace it with a subselect getting the equivalent data from pg_stat_io. It might not be quite worth doing for buffers_alloc because of the

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

2022-11-07 Thread Maciek Sakrejda
On Thu, Nov 3, 2022 at 10:00 AM Melanie Plageman wrote: > > I decided not to call it pg_statio because all of the other stats views > have an underscore after stat and I thought it was an opportunity to be > consistent with them. Oh, got it. Makes sense. > > I'm reviewing the rendered docs now,

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

2022-10-30 Thread Maciek Sakrejda
On Wed, Oct 26, 2022 at 10:55 AM Melanie Plageman wrote: + The pg_statio_ and + pg_stat_io views are primarily useful to determine + the effectiveness of the buffer cache. When the number of actual disk reads Totally nitpicking, but this reads a little funny to me. Previously the trailing

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

2022-10-26 Thread Andres Freund
Hi, On 2022-10-24 14:38:52 -0400, Melanie Plageman wrote: > > - "repossession" is a very unintuitive name for me. If we want something > > like > > it, can't we just name it reuse_failed or such? > > Repossession could be called eviction_failed or reuse_failed. > Do we think we will ever want

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

2022-10-26 Thread Melanie Plageman
okay, so I realized v35 had an issue where I wasn't counting strategy evictions correctly. fixed in attached v36. This made me wonder if there is actually a way to add a test for evictions (in strategy and shared contexts) that is not flakey. On Sun, Oct 23, 2022 at 6:48 PM Maciek Sakrejda

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

2022-10-24 Thread Melanie Plageman
On Sun, Oct 23, 2022 at 6:35 PM Maciek Sakrejda wrote: > > On Wed, Oct 19, 2022 at 12:27 PM Melanie Plageman > wrote: > > > > v34 is attached. > > I think the column names need discussion. Also, the docs need more work > > (I added a lot of new content there). I could use feedback on the column

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

2022-10-24 Thread Melanie Plageman
On Thu, Oct 20, 2022 at 1:31 PM Andres Freund wrote: > > Hi, > > - we shouldn't do pgstat_count_io_op() while the buffer header lock is held, > if possible. I've changed this locally. It will be fixed in the next version I share. > > I wonder if we should add a "source" output argument to >

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

2022-10-23 Thread Maciek Sakrejda
On Thu, Oct 20, 2022 at 10:31 AM Andres Freund wrote: > - "repossession" is a very unintuitive name for me. If we want something like > it, can't we just name it reuse_failed or such? +1, I think "repossessed" is awkward. I think "reuse_failed" works, but no strong opinions on an alternate

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

2022-10-23 Thread Maciek Sakrejda
On Wed, Oct 19, 2022 at 12:27 PM Melanie Plageman wrote: > > v34 is attached. > I think the column names need discussion. Also, the docs need more work > (I added a lot of new content there). I could use feedback on the column > names and definitions and review/rephrasing ideas for the docs >

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

2022-10-20 Thread Andres Freund
Hi, - we shouldn't do pgstat_count_io_op() while the buffer header lock is held, if possible. I wonder if we should add a "source" output argument to StrategyGetBuffer(). Then nearly all the counting can happen in BufferAlloc(). - "repossession" is a very unintuitive name for me. If we

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

2022-10-16 Thread Maciek Sakrejda
On Thu, Oct 13, 2022 at 10:29 AM Melanie Plageman wrote: > I think that it makes sense to count both the initial buffers added to > the ring and subsequent shared buffers added to the ring (either when > the current strategy buffer is pinned or in use or when a bulkread > rejects dirty strategy

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

2022-10-13 Thread Melanie Plageman
On Mon, Oct 10, 2022 at 7:43 PM Maciek Sakrejda wrote: > > Thanks for working on this! Like Lukas, I'm excited to see more > visibility into important parts of the system like this. Thanks for taking another look! > > On Mon, Oct 10, 2022 at 11:49 AM Melanie Plageman > wrote: > > > > I've gone

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

2022-10-10 Thread Maciek Sakrejda
Thanks for working on this! Like Lukas, I'm excited to see more visibility into important parts of the system like this. On Mon, Oct 10, 2022 at 11:49 AM Melanie Plageman wrote: > > I've gone ahead and implemented option 1 (commented below). No strong opinion on 1 versus 2, but I guess at least

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

2022-10-10 Thread Melanie Plageman
I've gone ahead and implemented option 1 (commented below). On Thu, Oct 6, 2022 at 6:23 PM Melanie Plageman wrote: > > v31 failed in CI, so > I've attached v32 which has a few issues fixed: > - addressed some compiler warnings I hadn't noticed locally > - autovac launcher and worker do indeed

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

2022-10-06 Thread Melanie Plageman
v31 failed in CI, so I've attached v32 which has a few issues fixed: - addressed some compiler warnings I hadn't noticed locally - autovac launcher and worker do indeed use bulkread strategy if they end up starting before critical indexes have loaded and end up doing a sequential scan of some

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

2022-10-06 Thread Melanie Plageman
v31 attached I've also addressed failing test mentioned by Andres in [1] On Fri, Sep 30, 2022 at 7:18 PM Lukas Fittl wrote: > > On Tue, Sep 27, 2022 at 11:20 AM Melanie Plageman > wrote: > > First of all, I'm excited about this patch, and I think it will be a big help > to understand better

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

2022-10-02 Thread Andres Freund
Hi, On 2022-09-27 14:20:44 -0400, Melanie Plageman wrote: > v30 attached > rebased and pgstat_io_ops.c builds with meson now > also, I tested with pgstat_report_stat() only flushing when forced and > tests still pass Unfortunately tests fail in CI / cfbot. E.g.,

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

2022-09-30 Thread Lukas Fittl
On Tue, Sep 27, 2022 at 11:20 AM Melanie Plageman wrote: > v30 attached > rebased and pgstat_io_ops.c builds with meson now > also, I tested with pgstat_report_stat() only flushing when forced and > tests still pass > First of all, I'm excited about this patch, and I think it will be a big help

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

2022-09-27 Thread Melanie Plageman
v30 attached rebased and pgstat_io_ops.c builds with meson now also, I tested with pgstat_report_stat() only flushing when forced and tests still pass From 153b06200b36891c9e07df526a86dbd913e36e3e Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 11 Aug 2022 18:28:50 -0400 Subject:

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

2022-08-25 Thread Andres Freund
Hi, On 2022-08-22 13:15:18 -0400, Melanie Plageman wrote: > v28 attached. > > I've added the new structs I added to typedefs.list. > > I've split the commit which adds all of the logic to track > IO operation statistics into two commits -- one which includes all of > the code to count IOOps for

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

2022-08-22 Thread Andres Freund
Hi, On 2022-08-22 13:15:18 -0400, Melanie Plageman wrote: > v28 attached. Pushed 0001, 0002. Thanks! - Andres

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

2022-08-22 Thread Melanie Plageman
v28 attached. I've added the new structs I added to typedefs.list. I've split the commit which adds all of the logic to track IO operation statistics into two commits -- one which includes all of the code to count IOOps for IOContexts locally in a backend and a second which includes all of the

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

2022-08-11 Thread Melanie Plageman
I've attached v27 of the patch. I've renamed IOPATH to IOCONTEXT. I also have added assertions to confirm that unexpected statistics are not being accumulated. There are also assorted other cleanups and changes. It would be good to confirm that the rows being skipped and cells that are NULL in

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

2022-07-20 Thread Melanie Plageman
On Wed, Jul 20, 2022 at 12:50 PM Andres Freund wrote: > > On 2022-07-14 18:44:48 -0400, Melanie Plageman wrote: > > > @@ -1427,8 +1445,10 @@ pgstat_read_statsfile(void) > > FILE *fpin; > > int32 format_id; > > boolfound; > > +

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

2022-07-20 Thread Andres Freund
Hi, On 2022-07-14 18:44:48 -0400, Melanie Plageman wrote: > Subject: [PATCH v26 1/4] Add BackendType for standalone backends > Subject: [PATCH v26 2/4] Remove unneeded call to pgstat_report_wal() LGTM. > Subject: [PATCH v26 3/4] Track IO operation statistics > @@ -978,8 +979,17 @@

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

2022-07-15 Thread Andres Freund
Hi, On 2022-07-15 11:59:41 -0400, Melanie Plageman wrote: > I'm not sure about the idea of prefixing the IOOp and IOPath enums with > Pg_Stat. I could see them being used outside of statistics (though they > are defined in pgstat.h) +1 > From Andres: > > Quoting me (Melanie): > > > Introduce

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

2022-07-15 Thread Melanie Plageman
I am consolidating the various naming points from this thread into one email: >From Horiguchi-san: > A bit different thing, but I felt a little uneasy about some uses of > "pgstat_io_ops". IOOp looks like a neighbouring word of IOPath. On the > other hand, actually iopath is used as an attribute

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

2022-07-14 Thread Melanie Plageman
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

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

2022-07-13 Thread Melanie Plageman
Attached patch set is substantially different enough from previous versions that I kept it as a new patch set. Note that local buffer allocations are now correctly tracked. On Tue, Jul 12, 2022 at 1:01 PM Andres Freund wrote: > Hi, > > On 2022-07-12 12:19:06 -0400, Melanie Plageman wrote: > > >

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

2022-07-12 Thread Kyotaro Horiguchi
At Tue, 12 Jul 2022 19:18:22 -0700, Andres Freund wrote in > Hi, > > On 2022-07-13 11:00:07 +0900, Kyotaro Horiguchi wrote: > > I imagined to use B_INVALID as a kind of "default" partition, which > > accepts all unknown backend types. > > There shouldn't be any unknown backend types. Something

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

2022-07-12 Thread Andres Freund
Hi, On 2022-07-13 11:00:07 +0900, Kyotaro Horiguchi wrote: > I imagined to use B_INVALID as a kind of "default" partition, which > accepts all unknown backend types. There shouldn't be any unknown backend types. Something has gone wrong if we get far without a backend type set. > We can just

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

2022-07-12 Thread Kyotaro Horiguchi
At Tue, 12 Jul 2022 12:19:06 -0400, Melanie Plageman wrote in > > + > > >io_ops.stats[backend_type_get_idx(MyBackendType)]; > > > > backend_type_get_idx(x) is actually (x - 1) plus assertion on the > > value range. And the only use-case is here. There's an reverse > > function and also used

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

2022-07-12 Thread Andres Freund
Hi, 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. > >

  1   2   >