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


Reply via email to