Horton

Thread

Commits

  1. Improve some GUC description strings

  2. Improve some error messages.

  1. A question about wording in messages

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-09-14T02:15:07Z

    I saw the following message recently modified.

> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.

Maybe the "we" means "PostgreSQL program and you" but I see it
somewhat out of place.

I found other three uses of "we" in backend.

> client sent proto_version=%d but we only support protocol %d or lower
> client sent proto_version=%d but we only support protocol %d or higher
> System allows %d, we need at least %d.

This is a little different from the first one. In the three above,
"we" suggests "The developers and maybe the PostgreSQL program".

Is it the right word choice as error messages?  I'm not confident on
the precise wording, but I think something like the following are
appropriate here.

> This controls the maximum distance to read ahead in the WAL to prefetch referenced data blocks.
> client sent proto_version=%d but only protocols %d or lower are supported
> client sent proto_version=%d but only protocols %d or higher are supported
> System allows %d, at least %d needed.

Thoughts?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  2. Re: A question about wording in messages

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-14T02:38:46Z

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> I saw the following message recently modified.
>> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
> Maybe the "we" means "PostgreSQL program and you" but I see it
> somewhat out of place.

+1, I saw that today and thought it was outside our usual style.
The whole thing is awfully verbose for a GUC description, too.
Maybe

"Maximum distance to read ahead in WAL to prefetch data blocks."

			regards, tom lane

  3. Re: A question about wording in messages

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-09-14T04:00:14Z

    At Tue, 13 Sep 2022 22:38:46 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > I saw the following message recently modified.
> >> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
> > Maybe the "we" means "PostgreSQL program and you" but I see it
> > somewhat out of place.
> 
> +1, I saw that today and thought it was outside our usual style.
> The whole thing is awfully verbose for a GUC description, too.
> Maybe
> 
> "Maximum distance to read ahead in WAL to prefetch data blocks."

It seems to sufficiently work for average users and rather easy to
read, but it looks a short description.

wal_decode_buffer_size has the following descriptions.

Short: Buffer size for reading ahead in the WAL during recovery.
Extra: This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.

So, taking the middle of them, how about the following?

Short: Buffer size for reading ahead in the WAL during recovery.
Extra: This controls the maximum distance to read ahead in WAL to prefetch data blocks."

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  4. Re: A question about wording in messages

    Amit Kapila <amit.kapila16@gmail.com> — 2022-09-14T06:39:55Z

    On Wed, Sep 14, 2022 at 7:45 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> I saw the following message recently modified.
>
> > This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
>
> Maybe the "we" means "PostgreSQL program and you" but I see it
> somewhat out of place.
>
> I found other three uses of "we" in backend.
>
> > client sent proto_version=%d but we only support protocol %d or lower
> > client sent proto_version=%d but we only support protocol %d or higher

How about just replacing 'we' with 'server'?

> > System allows %d, we need at least %d.
>

Another possibility could be: "System allows %d, but at least %d are required."

> This is a little different from the first one. In the three above,
> "we" suggests "The developers and maybe the PostgreSQL program".
>
> Is it the right word choice as error messages?  I'm not confident on
> the precise wording, but I think something like the following are
> appropriate here.
>
> > This controls the maximum distance to read ahead in the WAL to prefetch referenced data blocks.
> > client sent proto_version=%d but only protocols %d or lower are supported
> > client sent proto_version=%d but only protocols %d or higher are supported
> > System allows %d, at least %d needed.
>

This could be another way to rewrite. Let us see if others have an
opinion on this.

-- 
With Regards,
Amit Kapila.

  5. Re: A question about wording in messages

    Thomas Munro <thomas.munro@gmail.com> — 2022-09-16T00:10:05Z

    On Wed, Sep 14, 2022 at 2:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > I saw the following message recently modified.
> >> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
> > Maybe the "we" means "PostgreSQL program and you" but I see it
> > somewhat out of place.
>
> +1, I saw that today and thought it was outside our usual style.
> The whole thing is awfully verbose for a GUC description, too.
> Maybe
>
> "Maximum distance to read ahead in WAL to prefetch data blocks."

+1

For "we", I must have been distracted by code comment style.  For the
extra useless verbiage, it's common for GUC description to begin "This
control/affects/blah" like that, but I agree it's useless noise.

For the other cases, Amit's suggestion of 'server' seems sensible to me.

  6. Re: A question about wording in messages

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-09-16T06:16:15Z

    At Fri, 16 Sep 2022 12:10:05 +1200, Thomas Munro <thomas.munro@gmail.com> wrote in 
> On Wed, Sep 14, 2022 at 2:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > > I saw the following message recently modified.
> > >> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
> > > Maybe the "we" means "PostgreSQL program and you" but I see it
> > > somewhat out of place.
> >
> > +1, I saw that today and thought it was outside our usual style.
> > The whole thing is awfully verbose for a GUC description, too.
> > Maybe
> >
> > "Maximum distance to read ahead in WAL to prefetch data blocks."
> 
> +1
> 
> For "we", I must have been distracted by code comment style.  For the
> extra useless verbiage, it's common for GUC description to begin "This
> control/affects/blah" like that, but I agree it's useless noise.
> 
> For the other cases, Amit's suggestion of 'server' seems sensible to me.

Thaks for the opinion. I'm fine with that, too.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  7. Re: A question about wording in messages

    Amit Kapila <amit.kapila16@gmail.com> — 2022-09-16T06:59:52Z

    On Fri, Sep 16, 2022 at 11:46 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Fri, 16 Sep 2022 12:10:05 +1200, Thomas Munro <thomas.munro@gmail.com> wrote in
> > On Wed, Sep 14, 2022 at 2:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > > > I saw the following message recently modified.
> > > >> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
> > > > Maybe the "we" means "PostgreSQL program and you" but I see it
> > > > somewhat out of place.
> > >
> > > +1, I saw that today and thought it was outside our usual style.
> > > The whole thing is awfully verbose for a GUC description, too.
> > > Maybe
> > >
> > > "Maximum distance to read ahead in WAL to prefetch data blocks."
> >
> > +1
> >
> > For "we", I must have been distracted by code comment style.  For the
> > extra useless verbiage, it's common for GUC description to begin "This
> > control/affects/blah" like that, but I agree it's useless noise.
> >
> > For the other cases, Amit's suggestion of 'server' seems sensible to me.
>
> Thaks for the opinion. I'm fine with that, too.
>

So, the change related to wal_decode_buffer_size needs to be
backpatched to 15 whereas other message changes will be HEAD only, am
I correct?

-- 
With Regards,
Amit Kapila.

  8. Re: A question about wording in messages

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-09-16T08:37:08Z

    At Fri, 16 Sep 2022 12:29:52 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in 
> So, the change related to wal_decode_buffer_size needs to be
> backpatched to 15 whereas other message changes will be HEAD only, am
> I correct?

Has 15 closed the entry?  IMHO I supposed that all changes are applied
back(?) to 15.

regardes.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  9. Re: A question about wording in messages

    Amit Kapila <amit.kapila16@gmail.com> — 2022-09-16T08:58:34Z

    On Fri, Sep 16, 2022 at 2:07 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Fri, 16 Sep 2022 12:29:52 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in
> > So, the change related to wal_decode_buffer_size needs to be
> > backpatched to 15 whereas other message changes will be HEAD only, am
> > I correct?
>
> Has 15 closed the entry?  IMHO I supposed that all changes are applied
> back(?) to 15.
>

We only want to commit the changes to 15 (a) if those fixes a problem
introduced in 15, or (b) those are for a bug fix. I think the error
message improvements fall into none of those categories, we can map it
to (b) but I feel those are an improvement in the current messages and
don't seem critical to me.

-- 
With Regards,
Amit Kapila.

  10. Re: A question about wording in messages

    Amit Kapila <amit.kapila16@gmail.com> — 2022-09-20T04:30:40Z

    On Fri, Sep 16, 2022 at 12:29 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > > >
> > > > +1, I saw that today and thought it was outside our usual style.
> > > > The whole thing is awfully verbose for a GUC description, too.
> > > > Maybe
> > > >
> > > > "Maximum distance to read ahead in WAL to prefetch data blocks."
> > >
> > > +1
> > >
> > > For "we", I must have been distracted by code comment style.  For the
> > > extra useless verbiage, it's common for GUC description to begin "This
> > > control/affects/blah" like that, but I agree it's useless noise.
> > >
> > > For the other cases, Amit's suggestion of 'server' seems sensible to me.
> >
> > Thaks for the opinion. I'm fine with that, too.
> >
>
> So, the change related to wal_decode_buffer_size needs to be
> backpatched to 15 whereas other message changes will be HEAD only, am
> I correct?
>

I would like to pursue as per above unless there is more feedback on this.

-- 
With Regards,
Amit Kapila.

  11. Re: A question about wording in messages

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-09-20T16:21:25Z

    On 2022-Sep-14, Kyotaro Horiguchi wrote:

> At Tue, 13 Sep 2022 22:38:46 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> > Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > > I saw the following message recently modified.
> > >> This controls the maximum distance we can read ahead in the WAL to prefetch referenced data blocks.
> > > Maybe the "we" means "PostgreSQL program and you" but I see it
> > > somewhat out of place.
> > 
> > +1, I saw that today and thought it was outside our usual style.
> > The whole thing is awfully verbose for a GUC description, too.
> > Maybe
> > 
> > "Maximum distance to read ahead in WAL to prefetch data blocks."

I failed to notice this issue.  I agree it's unusual and +1 for changing it.

> It seems to sufficiently work for average users and rather easy to
> read, but it looks a short description.

> So, taking the middle of them, how about the following?
> 
> Short: Buffer size for reading ahead in the WAL during recovery.
> Extra: This controls the maximum distance to read ahead in WAL to prefetch data blocks."

But why do we care that it's short?  We don't need it to be long .. we
only need it to explain what it needs to explain.

After spending way too much time editing this line, I ended up with
exactly what Tom proposed, so +1 for his version.  I think "This
controls" adds nothing very useful, and we don't have it anywhere else,
except tcp_keepalives_count from where I also propose to remove it.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)

  12. Re: A question about wording in messages

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-09-20T17:28:35Z

    On 2022-Sep-16, Amit Kapila wrote:

> We only want to commit the changes to 15 (a) if those fixes a problem
> introduced in 15, or (b) those are for a bug fix. I think the error
> message improvements fall into none of those categories, we can map it
> to (b) but I feel those are an improvement in the current messages and
> don't seem critical to me.

IMO at least the GUC one does fix a problem related to the wording of a
user-visible message, which also flows into the translations.  I prefer
to have that one fixed it in 15 also.  The other messages (errors) don't
seem very interesting because they're not as visible, so I don't care if
those are not backpatched.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"This is a foot just waiting to be shot"                (Andrew Dunstan)

  13. Re: A question about wording in messages

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-20T18:56:47Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> After spending way too much time editing this line, I ended up with
> exactly what Tom proposed, so +1 for his version.  I think "This
> controls" adds nothing very useful, and we don't have it anywhere else,
> except tcp_keepalives_count from where I also propose to remove it.

LGTM.

			regards, tom lane