On Tue, Apr 7, 2020 at 2:48 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > > On Tue, Apr 7, 2020 at 4:36 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Apr 6, 2020 at 7:58 PM Euler Taveira > > <euler.tave...@2ndquadrant.com> wrote: > > > > > > On Mon, 6 Apr 2020 at 10:37, Julien Rouhaud <rjuju...@gmail.com> wrote: > > >> > > >> On Mon, Apr 06, 2020 at 10:12:55AM -0300, Euler Taveira wrote: > > >> > On Mon, 6 Apr 2020 at 00:25, Amit Kapila <amit.kapil...@gmail.com> > > >> > wrote: > > >> > > > >> > > > > >> > > I have pushed pg_stat_statements and Explain related patches. I am > > >> > > now looking into (auto)vacuum patch and have few comments. > > >> > > > > >> > > I wasn't paying much attention to this thread. May I suggest changing > > >> > wal_num_fpw to wal_fpw? wal_records and wal_bytes does not have a > > >> > prefix > > >> > 'num'. It seems inconsistent to me. > > >> > > > >> > > >> If we want to be consistent shouldn't we rename it to wal_fpws? FTR I > > >> don't > > >> like much either version. > > > > > > > > > Since FPW is an acronym, plural form reads better when you are using > > > uppercase (such as FPWs or FPW's); thus, I prefer singular form because > > > parameter names are lowercase. Function description will clarify that > > > this is "number of WAL full page writes". > > > > > > > I like Euler's suggestion to change wal_num_fpw to wal_fpw. It is > > better if others who didn't like this name can also share their > > opinion now because changing multiple times the same thing is not a > > good idea. > > +1 > > About Justin and your comments on the other thread: > > On Tue, Apr 7, 2020 at 4:31 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Apr 6, 2020 at 10:04 PM Justin Pryzby <pry...@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. > > Agreed. That's pg_stat_statements.c and instrument.h. I'll send a > patch once we reach consensus with the rest of the comments. >
Would you like to send a consolidated patch that includes Euler's suggestion and Justin's patch (by making changes for points we discussed.)? I think we can keep the point related to number of spaces before each field open? > > 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? > > I also prefer "amount" as it feels more natural. > As we see no other opinion on this matter, we can use "amount" here. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
