At Thu, 2 Apr 2020 00:41:20 -0500, Justin Pryzby <pry...@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
