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. > 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. I'm not a native english speaker though, so maybe I'm just biased.