User Interface for WAL usage data

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=12345 dirtied=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=12345 dirtied=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=12345 dirtied=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 records generated */
doc/src/sgml/ref/explain.sgml: number of records, number of full page image records and amount of WAL

--
Justin

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 records generated */
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]/messages/by-id/CAB-hujrP8ZfUkvL5OYETipQwA=e3n7oqHFU=4ZLxWS_Cza3kQQ@mail.gmail.com?

[1]: /messages/by-id/CAB-hujrP8ZfUkvL5OYETipQwA=e3n7oqHFU=4ZLxWS_Cza3kQQ@mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com