Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3776.700.51.11.1\))
Subject: Re: Moving the vacuum GUCs' docs out of the Client Connection
 Defaults section
From: Daniel Gustafsson <daniel@yesql.se>
In-Reply-To: 
 <CAAKRu_a52vCax5+AN7MstfMckuR1qxyfTmssyPhqZv+9mNiMGQ@mail.gmail.com>
Date: Fri, 10 Jan 2025 23:59:51 +0100
Cc: Peter Eisentraut <peter@eisentraut.org>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 "Bossart, Nathan" <bossartn@amazon.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <77916C10-F124-4652-B842-92508E1696B6@yesql.se>
References: 
 <CAAKRu_aQUOaMYrcjNuXeSkJtaX9oRUzKP57bsYbC0gVVWS+cbA@mail.gmail.com>
 <1373018.1736213217@sss.pgh.pa.us>
 <CAAKRu_bz3BD4w0jmjfQkckAQE5eb-Ge2a=9Q-HG_rWSTeG_PzA@mail.gmail.com>
 <CAAKRu_Ymux_RBzp5faSR5-qx1JdM6qS3POVjcWyHbnQjzgmo8g@mail.gmail.com>
 <68a4435c-3990-419d-a68a-b33a7f9185ab@eisentraut.org>
 <CAAKRu_ZR8aqiZ0SHK8Rqd7QVAmr6XX9Xpkie9vmZAkyb69YOiQ@mail.gmail.com>
 <ED2C0322-DA08-4B60-8A08-C69128182C68@yesql.se>
 <CAAKRu_a52vCax5+AN7MstfMckuR1qxyfTmssyPhqZv+9mNiMGQ@mail.gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/77916C10-F124-4652-B842-92508E1696B6%40yesql.se>
Precedence: bulk

> On 10 Jan 2025, at 23:09, Melanie Plageman <melanieplageman@gmail.com> =
wrote:
>=20
> On Thu, Jan 9, 2025 at 5:05=E2=80=AFPM Daniel Gustafsson =
<daniel@yesql.se> wrote:
>>=20
>> 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.
>>=20
>> +    <sect2 id=3D"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 =3D)
>=20
> I've taken a stab at improving this. Let me know if you think it =
works.

I like what you added, it's IMO the right level of detail here.

>> +         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".
>=20
> 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.

LGTM.  We're not entirely consistent with how we mark up the default =
value, but
I think moving towards using <literal>value</literal> whenever we are =
changing
things there anyways is the right level of cleanup.

>> +         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?
>=20
> 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.

To be fair I didn't really expect this to be changed as part of this =
(and I
should written that in my email), but it stood out when reading so =
wanted to
point it out.  No objections to leaving it as is as part of this work.

+         Specifies the cutoff age (in multixacts) that =
<command>VACUUM</command>
I wish we had a glossary entry for multixact we could link to here.  =
But,
that's not really the responsibility of this patch to fix, just =
something that
came to mind when reading the resulting page.

+1 on going ahead with this version, there are still improvements we can =
make
to the vacuum config docs but that shouldn't stand in the way of a reorg =
patch
which improves overall presentation structure.

--
Daniel Gustafsson