Thread: A question about wording in messages
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
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
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
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.
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.
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
Attachment
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.
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
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.
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.
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)
Attachment
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)
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