Thread: User Interface for WAL usage data
Hi,
In thread [1], we are discussing to expose WAL usage data for each
statement in a way quite similar to how we expose BufferUsage data.
The way it exposes seems reasonable to me and no one else raises any
objection. It could be that it appears fine to others who have
reviewed the patch but I thought it would be a good idea to write a
separate email just for its UI and see if anybody has objection.
It exposes three variables (a) wal_records (Number of WAL records
produced), (b) wal_num_fpw (Number of WAL full page image records),
(c) wal_bytes (size of WAL records produced).
The patch has exposed these three variables via explain (analyze, wal)
<statement>, auto_explain and pg_stat_statements.
Exposed via Explain
------------------------------------
Note the usage via line displaying WAL. This parameter may only be
used when ANALYZE is also enabled.
postgres=# explain (analyze, buffers, wal) update t1 set c2='cccc';
QUERY PLAN
-----------------------------------------------------------------------------------------------------------
Update on t1 (cost=0.00..53.99 rows=1199 width=414) (actual
time=6.030..6.030 rows=0 loops=1)
Buffers: shared hit=2484 dirtied=44
WAL: records=2359 full page records=42 bytes=447788
-> Seq Scan on t1 (cost=0.00..53.99 rows=1199 width=414) (actual
time=0.040..0.540 rows=1199 loops=1)
Buffers: shared hit=42
Planning Time: 0.179 ms
Execution Time: 6.119 ms
(7 rows)
Exposed via auto_explain
------------------------------------------
Users need to set auto_explain.log_wal to print WAL usage statistics.
This parameter has no effect unless auto_explain.log_analyze is
enabled. Note the usage via line displaying WAL.
LOG: duration: 0.632 ms plan:
Query Text: update t1 set c2='cccc';
Update on t1 (cost=0.00..16.10 rows=610 width=414) (actual
time=0.629..0.629 rows=0 loops=1)
Buffers: shared hit=206 dirtied=5 written=2
WAL: records=200 full page records=2 bytes=37387
-> Seq Scan on t1 (cost=0.00..16.10 rows=610 width=414) (actual
time=0.022..0.069 rows=100 loops=1)
Buffers: shared hit=2 dirtied=1
Exposed via pg_stat_statements
------------------------------------------------
Three new parameters are added to pg_stat_statements function.
select query, wal_bytes, wal_records, wal_num_fpw from
pg_stat_statements where query like 'VACUUM%';
query | wal_bytes | wal_records | wal_num_fpw
--------------------------+-----------+-------------+-------------
VACUUM test | 72814331 | 8857 | 8855
Any objections/suggestions?
[1] - https://www.postgresql.org/message-id/CAB-hujrP8ZfUkvL5OYETipQwA%3De3n7oqHFU%3D4ZLxWS_Cza3kQQ%40mail.gmail.com
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Thu, Apr 02, 2020 at 10:13:18AM +0530, Amit Kapila wrote:
> In thread [1], we are discussing to expose WAL usage data for each
> statement in a way quite similar to how we expose BufferUsage data.
> The way it exposes seems reasonable to me and no one else raises any
> objection. It could be that it appears fine to others who have
> reviewed the patch but I thought it would be a good idea to write a
> separate email just for its UI and see if anybody has objection.
+1
Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch:
I think there should be additional spaces before "full" and before "bytes":
> WAL: records=2359 full page records=42 bytes=447788
Compare with these:
"Sort Method: %s %s: %ldkB\n",
"Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",
"Buckets: %d Batches: %d Memory Usage: %ldkB\n",
Otherwise "records=2359 full page records=42" is hard to parse.
> Exposed via auto_explain
> WAL: records=200 full page records=2 bytes=37387
Same
In v10-0002:
+ * BufferUsage and WalUsage during executing maintenance command can be
should say "during execution of a maintenance command".
I'm afraid that'll cause merge conflicts for you :(
In 0003:
+ /* Provide WAL update data to the instrumentation */
Remove "data" ??
--
Justin
At Thu, 2 Apr 2020 00:41:20 -0500, Justin Pryzby <pryzby@telsasoft.com> wrote in > On Thu, Apr 02, 2020 at 10:13:18AM +0530, Amit Kapila wrote: > > In thread [1], we are discussing to expose WAL usage data for each > > statement in a way quite similar to how we expose BufferUsage data. > > The way it exposes seems reasonable to me and no one else raises any > > objection. It could be that it appears fine to others who have > > reviewed the patch but I thought it would be a good idea to write a > > separate email just for its UI and see if anybody has objection. > > +1 > > Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch: > I think there should be additional spaces before "full" and before "bytes": > > > WAL: records=2359 full page records=42 bytes=447788 > > Compare with these: > > "Sort Method: %s %s: %ldkB\n", > "Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n", > "Buckets: %d Batches: %d Memory Usage: %ldkB\n", > > Otherwise "records=2359 full page records=42" is hard to parse. I got the same feeling seeing the line. "full page records" seems to be showing the number of full page images, not the record having full page images. > > Exposed via auto_explain > > WAL: records=200 full page records=2 bytes=37387 > > Same > > In v10-0002: > + * BufferUsage and WalUsage during executing maintenance command can be > should say "during execution of a maintenance command". > I'm afraid that'll cause merge conflicts for you :( > > In 0003: > + /* Provide WAL update data to the instrumentation */ > Remove "data" ?? -- Kyotaro Horiguchi NTT Open Source Software Center
On Thu, Apr 2, 2020 at 11:28 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > > At Thu, 2 Apr 2020 00:41:20 -0500, Justin Pryzby <pryzby@telsasoft.com> wrote in > > On Thu, Apr 02, 2020 at 10:13:18AM +0530, Amit Kapila wrote: > > > In thread [1], we are discussing to expose WAL usage data for each > > > statement in a way quite similar to how we expose BufferUsage data. > > > The way it exposes seems reasonable to me and no one else raises any > > > objection. It could be that it appears fine to others who have > > > reviewed the patch but I thought it would be a good idea to write a > > > separate email just for its UI and see if anybody has objection. > > > > +1 > > > > Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch: > > I think there should be additional spaces before "full" and before "bytes": > > > > > WAL: records=2359 full page records=42 bytes=447788 > > > > Compare with these: > > > > "Sort Method: %s %s: %ldkB\n", > > "Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n", > > "Buckets: %d Batches: %d Memory Usage: %ldkB\n", > > > > Otherwise "records=2359 full page records=42" is hard to parse. > > I got the same feeling seeing the line. > But isn't this same as we have BufferUsage data? We can probably display it as full_page_writes or something like that. > "full page records" seems to be showing the number of full page > images, not the record having full page images. > I am not sure what exactly is a difference but it is the records having full page images. Julien correct me if I am wrong. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Thanks all for the feedback.
On Thu, Apr 2, 2020 at 8:02 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Apr 2, 2020 at 11:28 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > At Thu, 2 Apr 2020 00:41:20 -0500, Justin Pryzby <pryzby@telsasoft.com> wrote in
> > > On Thu, Apr 02, 2020 at 10:13:18AM +0530, Amit Kapila wrote:
> > > > In thread [1], we are discussing to expose WAL usage data for each
> > > > statement in a way quite similar to how we expose BufferUsage data.
> > > > The way it exposes seems reasonable to me and no one else raises any
> > > > objection. It could be that it appears fine to others who have
> > > > reviewed the patch but I thought it would be a good idea to write a
> > > > separate email just for its UI and see if anybody has objection.
> > >
> > > +1
> > >
> > > Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch:
> > > I think there should be additional spaces before "full" and before "bytes":
> > >
> > > > WAL: records=2359 full page records=42 bytes=447788
> > >
> > > Compare with these:
> > >
> > > "Sort Method: %s %s: %ldkB\n",
> > > "Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",
> > > "Buckets: %d Batches: %d Memory Usage: %ldkB\n",
> > >
> > > Otherwise "records=2359 full page records=42" is hard to parse.
> >
> > I got the same feeling seeing the line.
> >
>
> But isn't this same as we have BufferUsage data? We can probably
> display it as full_page_writes or something like that.
>
> > "full page records" seems to be showing the number of full page
> > images, not the record having full page images.
> >
>
> I am not sure what exactly is a difference but it is the records
> having full page images. Julien correct me if I am wrong.
This counter should be showing the number of full page image included
in the WAL record(s). The goal is to try to estimate how much FPI are
amplifying WAL records for a given cumulated size of WAL data. I had
seen some pretty high amplification recently due to an autovacuum
freeze. Using pg_waldump to analyze a sample of ~ 100GB of WALs
showed that a 1.5% increase in the number of freeze records lead to
more than 15% increase on the total amount of WAL, due to a high
amount of those records being FPW.
Also note that the patchset Amit is referencing adds the same
instrumentation for vacuum (verbose) and autovacuum, although this
part is showing the intended "full page write" label rather than the
bogus "full page records". Example output:
LOG: automatic vacuum of table "rjuju.public.t1": index scans: 0
pages: 0 removed, 2213 remain, 0 skipped due to pins, 0 skipped frozen
tuples: 250000 removed, 250000 remain, 0 are dead but not yet
removable, oldest xmin: 502
buffer usage: 4448 hits, 4 misses, 4 dirtied
avg read rate: 0.160 MB/s, avg write rate: 0.160 MB/s
system usage: CPU: user: 0.13 s, system: 0.00 s, elapsed: 0.19 s
WAL usage: 6643 records, 4 full page writes, 1402679 bytes
VACUUM log sample:
# vacuum VERBOSE t1;
INFO: vacuuming "public.t1"
INFO: "t1": removed 50000 row versions in 443 pages
INFO: "t1": found 50000 removable, 0 nonremovable row versions in 443
out of 443 pages
DETAIL: 0 dead row versions cannot be removed yet, oldest xmin: 512
There were 50000 unused item identifiers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
1332 WAL records, 4 WAL full page writes, 306901 WAL bytes
CPU: user: 0.01 s, system: 0.00 s, elapsed: 0.01 s.
INFO: "t1": truncated 443 to 0 pages
DETAIL: CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s
INFO: vacuuming "pg_toast.pg_toast_16385"
INFO: index "pg_toast_16385_index" now contains 0 row versions in 1 pages
DETAIL: 0 index row versions were removed.
0 index pages have been deleted, 0 are currently reusable.
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
INFO: "pg_toast_16385": found 0 removable, 0 nonremovable row
versions in 0 out of 0 pages
DETAIL: 0 dead row versions cannot be removed yet, oldest xmin: 513
There were 0 unused item identifiers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
0 WAL records, 0 WAL full page writes, 0 WAL bytes
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
VACUUM
Obviously previous complaints about the meaning and parsability of
"full page writes" should be addressed here for consistency.
On Thu, Apr 02, 2020 at 11:32:16AM +0530, Amit Kapila wrote:
> On Thu, Apr 2, 2020 at 11:28 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> >
> > At Thu, 2 Apr 2020 00:41:20 -0500, Justin Pryzby <pryzby@telsasoft.com> wrote in
> > > Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch:
> > > I think there should be additional spaces before "full" and before "bytes":
> > >
> > > > WAL: records=2359 full page records=42 bytes=447788
> > >
> > > Compare with these:
> > >
> > > "Sort Method: %s %s: %ldkB\n",
> > > "Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",
> > > "Buckets: %d Batches: %d Memory Usage: %ldkB\n",
> > >
> > > Otherwise "records=2359 full page records=42" is hard to parse.
> >
> > I got the same feeling seeing the line.
>
> But isn't this same as we have BufferUsage data? We can probably
> display it as full_page_writes or something like that.
I guess you mean this:
Buffers: shared hit=994 read=11426 dirtied=466
Which can show shared/local/temp. Actually I would probably make the same
suggestion for "Buffers" (if it were a new patch). I would find this to be
pretty unfriendly output:
Buffers: shared hit=12345 read=12345 dirtied=12345 local hit=12345 read=12345 dirtied=12345 temp hit=12345
read=12345dirtied=12345
Adding two extra spaces " local" and " temp" would have helped there, so
would commas, or parenthesis, dashes or almost anything - other than a
backslash.
So I think you're right that WAL is very similar to the Buffers case, but I
suggest that's not a good example to follow, especially since you're adding a
"field" with spaces in it.
I thought maybe the "two spaces" convention predated "Buffers". But sort has
had two spaces since it was added 2009-08-10 (9bd27b7c9). Buffers since it was
added 2009-12-15 (cddca5ec). And buckets since it was added 2010-02-01
(42a8ab0a).
--
Justin
On Thu, Apr 2, 2020 at 12:05 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > > On Thu, Apr 02, 2020 at 11:32:16AM +0530, Amit Kapila wrote: > > On Thu, Apr 2, 2020 at 11:28 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > > > > > > At Thu, 2 Apr 2020 00:41:20 -0500, Justin Pryzby <pryzby@telsasoft.com> wrote in > > > > Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch: > > > > I think there should be additional spaces before "full" and before "bytes": > > > > > > > > > WAL: records=2359 full page records=42 bytes=447788 > > > > > > > > Compare with these: > > > > > > > > "Sort Method: %s %s: %ldkB\n", > > > > "Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n", > > > > "Buckets: %d Batches: %d Memory Usage: %ldkB\n", > > > > > > > > Otherwise "records=2359 full page records=42" is hard to parse. > > > > > > I got the same feeling seeing the line. > > > > But isn't this same as we have BufferUsage data? We can probably > > display it as full_page_writes or something like that. > > I guess you mean this: > Buffers: shared hit=994 read=11426 dirtied=466 > > Which can show shared/local/temp. Actually I would probably make the same > suggestion for "Buffers" (if it were a new patch). I would find this to be > pretty unfriendly output: > > Buffers: shared hit=12345 read=12345 dirtied=12345 local hit=12345 read=12345 dirtied=12345 temp hit=12345 read=12345dirtied=12345 > > Adding two extra spaces " local" and " temp" would have helped there, so > would commas, or parenthesis, dashes or almost anything - other than a > backslash. > > So I think you're right that WAL is very similar to the Buffers case, but I > suggest that's not a good example to follow, especially since you're adding a > "field" with spaces in it. > Agreed, this is a good reason to keep two spaces as we have in cases of Buckets. We can display the variable as "full page writes". -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
> > > > > Regarding v10-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au.patch:
> > > > > I think there should be additional spaces before "full" and before "bytes":
> > > > >
> > > > > > WAL: records=2359 full page records=42 bytes=447788
> > > > >
> > > > > Compare with these:
> > > > >
> > > > > "Sort Method: %s %s: %ldkB\n",
> > > > > "Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",
> > > > > "Buckets: %d Batches: %d Memory Usage: %ldkB\n",
> > > > >
> > > > > Otherwise "records=2359 full page records=42" is hard to parse.
> > > >
> > > > I got the same feeling seeing the line.
> > >
> > > But isn't this same as we have BufferUsage data? We can probably
> > > display it as full_page_writes or something like that.
> >
> > I guess you mean this:
> > Buffers: shared hit=994 read=11426 dirtied=466
> >
> > Which can show shared/local/temp. Actually I would probably make the same
> > suggestion for "Buffers" (if it were a new patch). I would find this to be
> > pretty unfriendly output:
> >
> > Buffers: shared hit=12345 read=12345 dirtied=12345 local hit=12345 read=12345 dirtied=12345 temp hit=12345
read=12345dirtied=12345
> >
> > Adding two extra spaces " local" and " temp" would have helped there, so
> > would commas, or parenthesis, dashes or almost anything - other than a
> > backslash.
> >
> > So I think you're right that WAL is very similar to the Buffers case[...]
Actually, I take that back. If I understand correctly, there should be two
spaces not only to make the 2nd field more clear, but because the three values
have different units:
> > > > > > WAL: records=2359 full page records=42 bytes=447788
1) records; 2) pages ("full page images"); 3) bytes
That is exactly like sort (method/type/size) and hash (buckets/batches/size),
and *not* like buffers, which shows various values all in units of "pages".
--
Justin
On Fri, Apr 3, 2020 at 10:41 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> > > > > > > WAL: records=2359 full page records=42 bytes=447788
>
> 1) records; 2) pages ("full page images"); 3) bytes
>
> That is exactly like sort (method/type/size) and hash (buckets/batches/size),
> and *not* like buffers, which shows various values all in units of "pages".
>
The way you have written (2) appears to bit awkward. I would prefer
"full page writes" or "full page images".
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 03, 2020 at 10:52:02AM +0530, Amit Kapila wrote:
> On Fri, Apr 3, 2020 at 10:41 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > > > > > > > WAL: records=2359 full page records=42 bytes=447788
> >
> > 1) records; 2) pages ("full page images"); 3) bytes
> >
> > That is exactly like sort (method/type/size) and hash (buckets/batches/size),
> > and *not* like buffers, which shows various values all in units of "pages".
> >
>
> The way you have written (2) appears to bit awkward. I would prefer
> "full page writes" or "full page images".
I didn't mean it to be the description used in the patch or anywhere else, just
the list of units.
I wonder if it should use colons instead of equals ? As in:
| WAL: Records: 2359 Full Page Images: 42 Size: 437kB
Note, that has: 1) two spaces; 2) capitalized "fields"; 3) size rather than
"bytes". That's similar to Buckets:
| Buckets: 1024 Batches: 1 Memory Usage: 44kB
I'm not sure if it should say "WAL: " or "WAL ", or perhaps "WAL: " If
there's no colon, then it looks like the first field is "WAL Records", but then
"size" isn't as tightly associated with WAL. It could say:
| WAL Records: n Full Page Images: n WAL Size: nkB
For comparison, buffers uses "equals" for the case showing multiple "fields",
which are all in units of pages:
| Buffers: shared hit=15 read=2006
Also, for now, the output can be in kB, but I think in the future we should
take a recent suggestion from Andres to make an ExplainPropertyBytes() which
handles conversion to and display of a reasonable unit.
--
Justin
On Fri, Apr 3, 2020 at 11:14 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Fri, Apr 03, 2020 at 10:52:02AM +0530, Amit Kapila wrote:
> > On Fri, Apr 3, 2020 at 10:41 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > >
> > > > > > > > > WAL: records=2359 full page records=42 bytes=447788
> > >
> > > 1) records; 2) pages ("full page images"); 3) bytes
> > >
> > > That is exactly like sort (method/type/size) and hash (buckets/batches/size),
> > > and *not* like buffers, which shows various values all in units of "pages".
> > >
> >
> > The way you have written (2) appears to bit awkward. I would prefer
> > "full page writes" or "full page images".
>
> I didn't mean it to be the description used in the patch or anywhere else, just
> the list of units.
>
> I wonder if it should use colons instead of equals ? As in:
> | WAL: Records: 2359 Full Page Images: 42 Size: 437kB
>
> Note, that has: 1) two spaces; 2) capitalized "fields"; 3) size rather than
> "bytes". That's similar to Buckets:
> | Buckets: 1024 Batches: 1 Memory Usage: 44kB
>
> I'm not sure if it should say "WAL: " or "WAL ", or perhaps "WAL: " If
> there's no colon, then it looks like the first field is "WAL Records", but then
> "size" isn't as tightly associated with WAL. It could say:
> | WAL Records: n Full Page Images: n WAL Size: nkB
>
> For comparison, buffers uses "equals" for the case showing multiple "fields",
> which are all in units of pages:
> | Buffers: shared hit=15 read=2006
>
I think this is more close to the case of Buffers where all fields are
directly related to buffers/blocks. Here all the fields we want to
display are related to WAL, so we should try to make it display
similar to Buffers.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 3, 2020 at 11:29 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Apr 3, 2020 at 11:14 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Fri, Apr 03, 2020 at 10:52:02AM +0530, Amit Kapila wrote:
> > > On Fri, Apr 3, 2020 at 10:41 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > >
> > > > > > > > > > WAL: records=2359 full page records=42 bytes=447788
> > > >
> > > > 1) records; 2) pages ("full page images"); 3) bytes
> > > >
> > > > That is exactly like sort (method/type/size) and hash (buckets/batches/size),
> > > > and *not* like buffers, which shows various values all in units of "pages".
> > > >
> > >
> > > The way you have written (2) appears to bit awkward. I would prefer
> > > "full page writes" or "full page images".
> >
> > I didn't mean it to be the description used in the patch or anywhere else, just
> > the list of units.
> >
> > I wonder if it should use colons instead of equals ? As in:
> > | WAL: Records: 2359 Full Page Images: 42 Size: 437kB
> >
> > Note, that has: 1) two spaces; 2) capitalized "fields"; 3) size rather than
> > "bytes". That's similar to Buckets:
> > | Buckets: 1024 Batches: 1 Memory Usage: 44kB
> >
> > I'm not sure if it should say "WAL: " or "WAL ", or perhaps "WAL: " If
> > there's no colon, then it looks like the first field is "WAL Records", but then
> > "size" isn't as tightly associated with WAL. It could say:
> > | WAL Records: n Full Page Images: n WAL Size: nkB
> >
> > For comparison, buffers uses "equals" for the case showing multiple "fields",
> > which are all in units of pages:
> > | Buffers: shared hit=15 read=2006
> >
>
> I think this is more close to the case of Buffers where all fields are
> directly related to buffers/blocks. Here all the fields we want to
> display are related to WAL, so we should try to make it display
> similar to Buffers.
>
Dilip, Julien, others, do you have any suggestions here? I think we
need to decide something now. We can change a few things like from
'two spaces' to 'one space' between fields later as well.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 3, 2020 at 1:57 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Apr 3, 2020 at 11:29 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Apr 3, 2020 at 11:14 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > >
> > > On Fri, Apr 03, 2020 at 10:52:02AM +0530, Amit Kapila wrote:
> > > > On Fri, Apr 3, 2020 at 10:41 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > > >
> > > > > > > > > > > WAL: records=2359 full page records=42 bytes=447788
> > > > >
> > > > > 1) records; 2) pages ("full page images"); 3) bytes
> > > > >
> > > > > That is exactly like sort (method/type/size) and hash (buckets/batches/size),
> > > > > and *not* like buffers, which shows various values all in units of "pages".
> > > > >
> > > >
> > > > The way you have written (2) appears to bit awkward. I would prefer
> > > > "full page writes" or "full page images".
> > >
> > > I didn't mean it to be the description used in the patch or anywhere else, just
> > > the list of units.
> > >
> > > I wonder if it should use colons instead of equals ? As in:
> > > | WAL: Records: 2359 Full Page Images: 42 Size: 437kB
> > >
> > > Note, that has: 1) two spaces; 2) capitalized "fields"; 3) size rather than
> > > "bytes". That's similar to Buckets:
> > > | Buckets: 1024 Batches: 1 Memory Usage: 44kB
> > >
> > > I'm not sure if it should say "WAL: " or "WAL ", or perhaps "WAL: " If
> > > there's no colon, then it looks like the first field is "WAL Records", but then
> > > "size" isn't as tightly associated with WAL. It could say:
> > > | WAL Records: n Full Page Images: n WAL Size: nkB
> > >
> > > For comparison, buffers uses "equals" for the case showing multiple "fields",
> > > which are all in units of pages:
> > > | Buffers: shared hit=15 read=2006
> > >
> >
> > I think this is more close to the case of Buffers where all fields are
> > directly related to buffers/blocks. Here all the fields we want to
> > display are related to WAL, so we should try to make it display
> > similar to Buffers.
> >
>
> Dilip, Julien, others, do you have any suggestions here? I think we
> need to decide something now. We can change a few things like from
> 'two spaces' to 'one space' between fields later as well.
I also think it is more close to the BufferUsage so better to keep
similar to that. If we think the parsing is the problem we can keep
'_' in the multi-word name as shown below.
WAL: records=n full_page_writes=n bytes=n
And, all three fields are related to WAL so we can use WAL: followed
by other fields as we are doing now in the current patch.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 3, 2020 at 11:58 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Fri, Apr 3, 2020 at 1:57 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Apr 3, 2020 at 11:29 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Fri, Apr 3, 2020 at 11:14 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > >
> > > > On Fri, Apr 03, 2020 at 10:52:02AM +0530, Amit Kapila wrote:
> > > > > On Fri, Apr 3, 2020 at 10:41 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > > > >
> > > > > > > > > > > > WAL: records=2359 full page records=42 bytes=447788
> > > > > >
> > > > > > 1) records; 2) pages ("full page images"); 3) bytes
> > > > > >
> > > > > > That is exactly like sort (method/type/size) and hash (buckets/batches/size),
> > > > > > and *not* like buffers, which shows various values all in units of "pages".
> > > > > >
> > > > >
> > > > > The way you have written (2) appears to bit awkward. I would prefer
> > > > > "full page writes" or "full page images".
> > > >
> > > > I didn't mean it to be the description used in the patch or anywhere else, just
> > > > the list of units.
> > > >
> > > > I wonder if it should use colons instead of equals ? As in:
> > > > | WAL: Records: 2359 Full Page Images: 42 Size: 437kB
> > > >
> > > > Note, that has: 1) two spaces; 2) capitalized "fields"; 3) size rather than
> > > > "bytes". That's similar to Buckets:
> > > > | Buckets: 1024 Batches: 1 Memory Usage: 44kB
> > > >
> > > > I'm not sure if it should say "WAL: " or "WAL ", or perhaps "WAL: " If
> > > > there's no colon, then it looks like the first field is "WAL Records", but then
> > > > "size" isn't as tightly associated with WAL. It could say:
> > > > | WAL Records: n Full Page Images: n WAL Size: nkB
> > > >
> > > > For comparison, buffers uses "equals" for the case showing multiple "fields",
> > > > which are all in units of pages:
> > > > | Buffers: shared hit=15 read=2006
> > > >
> > >
> > > I think this is more close to the case of Buffers where all fields are
> > > directly related to buffers/blocks. Here all the fields we want to
> > > display are related to WAL, so we should try to make it display
> > > similar to Buffers.
> > >
> >
> > Dilip, Julien, others, do you have any suggestions here? I think we
> > need to decide something now. We can change a few things like from
> > 'two spaces' to 'one space' between fields later as well.
>
> I also think it is more close to the BufferUsage so better to keep
> similar to that.
+1 too for keeping consistency with BufferUsage, and adding extra
spaces if needed.
> If we think the parsing is the problem we can keep
> '_' in the multi-word name as shown below.
> WAL: records=n full_page_writes=n bytes=n
I'm fine with it too.
To answer Justin too:
> Also, for now, the output can be in kB, but I think in the future we should
> take a recent suggestion from Andres to make an ExplainPropertyBytes() which
> handles conversion to and display of a reasonable unit.
This could be nice, but I think that it raises some extra concerns.
There are multiple tools that parse those outputs, and having to deal
with a new and non-fixed units may cause some issues. And probably
the non text output would also need to be displayed differently.
On Thu, Apr 02, 2020 at 08:29:31AM +0200, Julien Rouhaud wrote: > > > "full page records" seems to be showing the number of full page > > > images, not the record having full page images. > > > > I am not sure what exactly is a difference but it is the records > > having full page images. Julien correct me if I am wrong. > Obviously previous complaints about the meaning and parsability of > "full page writes" should be addressed here for consistency. There's a couple places that say "full page image records" which I think is language you were trying to avoid. It's the number of pages, not the number of records, no ? I see explain and autovacuum say what I think is wanted, but these say the wrong thing? Find attached slightly larger patch. $ git grep 'image record' contrib/pg_stat_statements/pg_stat_statements.c: int64 wal_num_fpw; /* # of WAL full page image recordsgenerated */ doc/src/sgml/ref/explain.sgml: number of records, number of full page image records and amount of WAL -- Justin
Attachment
On Mon, Apr 6, 2020 at 10:04 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Thu, Apr 02, 2020 at 08:29:31AM +0200, Julien Rouhaud wrote:
> > > > "full page records" seems to be showing the number of full page
> > > > images, not the record having full page images.
> > >
> > > I am not sure what exactly is a difference but it is the records
> > > having full page images. Julien correct me if I am wrong.
>
> > Obviously previous complaints about the meaning and parsability of
> > "full page writes" should be addressed here for consistency.
>
> There's a couple places that say "full page image records" which I think is
> language you were trying to avoid. It's the number of pages, not the number of
> records, no ? I see explain and autovacuum say what I think is wanted, but
> these say the wrong thing? Find attached slightly larger patch.
>
> $ git grep 'image record'
> contrib/pg_stat_statements/pg_stat_statements.c: int64 wal_num_fpw; /* # of WAL full page image
recordsgenerated */
> doc/src/sgml/ref/explain.sgml: number of records, number of full page image records and amount of WAL
>
Few comments:
1.
- int64 wal_num_fpw; /* # of WAL full page image records generated */
+ int64 wal_num_fpw; /* # of WAL full page images generated */
Let's change comment as " /* # of WAL full page writes generated */"
to be consistent with other places like instrument.h. Also, make a
similar change at other places if required.
2.
<entry>
- Total amount of WAL bytes generated by the statement
+ Total number of WAL bytes generated by the statement
</entry>
I feel the previous text was better as this field can give us the size
of WAL with which we can answer "how much WAL data is generated by a
particular statement?". Julien, do you have any thoughts on this?
Can we please post/discuss patches on the main thread [1]?
[1] - https://www.postgresql.org/message-id/CAB-hujrP8ZfUkvL5OYETipQwA%3De3n7oqHFU%3D4ZLxWS_Cza3kQQ%40mail.gmail.com
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com