On Thu, Jan 9, 2025 at 5:05 PM Daniel Gustafsson <dan...@yesql.se> wrote:
>
> I think this is a really good restructuring which will make life easier for 
> our
> users.  Some of the comments below are on wording which wasn't introduced in
> this patch, but which I hadn't thought about before, so feel free to ignore
> those comments.
>
> +    <sect2 id="runtime-config-vacuum-freezing">
> +     <title>Freezing</title>
> +
> +     <para>
> Trying to read this as a new user, I think it would be good to start this
> subsection with a sentence describing what freezing actually is.  Vacuum is
> hard enough for users as it is =)

I've taken a stab at improving this. Let me know if you think it works.

> +         default value is one.
> Grepping around indicates that we typically use the numeric value rather than
> writing it in text, and the next settting down has "default value is 2".  For
> consistency I would change that to "1" instead of "one".

I think this is a reasonable cleanup to lump in with the rest of this
commit. I have taken the liberty of also adding a <literal> tag and
then updating the other places in the proposed "Vacuuming" subsection
where a literal default value is specified without the <literal> tag.

> +      <varname>vacuum_cost_delay</varname>. Then it will reset the counter 
> and
> +      continue execution.
> I know starting a sentence with "Then" is grammatically correct when it's the
> last sentence in a paragraph, but being a non-native speaker I always find
> myself re-reading such sentences to parse them as they stand out as odd.

I hear you. I couldn't think of something much clearer, so I left it as is.

> +         can be set to fractional-millisecond values, such delays may not be
> +         measured accurately on older platforms.  On such platforms,
> This sentence seems quite vague and hard to act on for users, what qualifies 
> as
> an "older platform" by the time v18 rolls around (this was added in v12).  I'm
> sure there are such platforms in existence that postgres 18 will run on, but
> are we helping users with ambiguity?

While I agree that what counted as older hardware in 2019 may no
longer be around at all, I am more hesitant to update this in the same
commit as a bunch of other cut-and-pastes. Someone at some point
decided this was important to point out, and I don't have sufficient
evidence that it no longer makes sense. And if I did, I'd probably
want to update this part of the docs in a dedicated commit.

- Melanie

Attachment: v3-0001-Consolidate-docs-for-vacuum-related-GUCs-in-new-s.patch
Description: Binary data

Reply via email to