Re: WAL usage calculation patch

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: rjuju123(at)gmail(dot)com
Cc: pryzby(at)telsasoft(dot)com, amit(dot)kapila16(at)gmail(dot)com, peter(dot)eisentraut(at)2ndquadrant(dot)com, dilipbalaut(at)gmail(dot)com, kuntalghosh(dot)2007(at)gmail(dot)com, masao(dot)fujii(at)oss(dot)nttdata(dot)com, kirill(dot)bychik(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, thomas(dot)munro(at)gmail(dot)com
Subject: Re: WAL usage calculation patch
Date: 2020-04-20 07:46:56
Message-ID: 20200420.164656.881770765734654652.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Sun, 19 Apr 2020 16:22:26 +0200, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote in
> Hi Justin,
>
> Thanks for the review!
>
> On Sat, Apr 18, 2020 at 10:41 PM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
> >
> > Should capitalize at least the non-text one ? And maybe the text one for
> > consistency ?
> >
> > + ExplainPropertyInteger("WAL fpw", NULL,
>
> I think we should keep both version consistent, whether lower or upper
> case. The uppercase version is probably more correct, but it's a
> little bit weird to have it being the only upper case label in all
> output, so I kept it lower case.

One space follwed by an acronym looks perfect. I'd prefer capital
letters but small-letters also works well.

> > And add the acronym to the docs:
> >
> > $ git grep 'full page' '*/explain.sgml'
> > doc/src/sgml/ref/explain.sgml: number of records, number of full page writes and amount of WAL bytes
> >
> > "..full page writes (FPW).."
>
> Indeed! Fixed (using lowercase to match current output).

I searched through the documentation and AFAICS most of occurances of
"full page" are follwed by "image" and full_page_writes is used only
as the parameter name.

I'm fine with fpw as the acronym, but "fpw means the number of full
page images" looks odd..

> > Should we also change vacuumlazy.c for consistency ?
> >
> > + _("WAL usage: %ld records, %ld full page writes, "
> > + UINT64_FORMAT " bytes"),
>
> I don't think this one should be changed, vacuumlazy output is already
> entirely different, and is way more verbose so keeping it as is makes
> sense to me.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2020-04-20 07:59:54 Re: 001_rep_changes.pl stalls
Previous Message Michael Paquier 2020-04-20 07:34:44 Re: [BUG] non archived WAL removed during production crash recovery