Re: First draft of PG 17 release notes

Hi,

On 2024-05-22 18:33:03 -0400, Bruce Momjian wrote:
> On Tue, May 21, 2024 at 09:40:28AM -0700, Andres Freund wrote:
> > On 2024-05-18 11:13:54 -0400, Bruce Momjian wrote:
> > I agree keeping things reasonably short is important. But I don't think you're
> > evenly applying it as a goal.
> >
> > Just skimming the notes from the end, I see
> > - an 8 entries long pg_stat_statements section
>
> What item did you want to remove?  Those are all user-visible changes.

My point here was not that we necessarily need to remove those, but that their
impact to users is smaller than many of the performance impacts you disregard.


> > - multiple entries about "Create custom wait events for ..."
>
> Well, those are all in different sections, so how can they be merged,
> unless I create a "wait event section", I guess.

They're not, all are in "Additional Modules". Instead of

- Create custom wait events for postgres_fdw (Masahiro Ikeda)
- Create custom wait events for dblink (Masahiro Ikeda)
- Allow extensions to define custom wait events (Masahiro Ikeda)

I'd make it:

- Allow extensions to define custom wait events and create custom wait events
  for postgres_fdw, dblink (Masahiro Ikeda)


> > - three entries about adding --all to {reindexdb,vacuumdb,clusterdb}.
>
> The problem with merging these is that the "Specifically, --all can now
> be used with" is different for all three of them.

You said you were worried about the length of the release notes, because it
discourages users from actually reading the release notes, due to getting
bored. Having three instance of almost the same entry, with just minor changes
between them, seems to precisely endanger boring readers.

I'd probably just go for

- Add --all option to clusterdb, reindexdb, vacuumdb to process objects in all
  databases matching a pattern (Nathan Bossart)

or such. The precise details of how the option works for the different
commands doesn't need to be stated in the release notes, that's more of a
reference documentation thing. But if you want to include it, we can do
something like

  Specifically, --all can now be used with --table (all commands), --schema
  (reindexdb, vacuumdb), and --exclude-schema (reindexdb, vacuumdb).


> > - an entry about adding long options to pg_archivecleanup
>
> Well, that is a user-visible change.  Should it not be listed?

If you are concerned about the length of the release notes and as a
consequence not including more impactful performance changes, then no, it
shouldn't. It doesn't break anyones current scripts, it doesn't enable
anything new.


> > - two entries about grantable maintenance rights, once via pg_maintain, once
> >   per-table
>
> Well, one is a GRANT and another is a role, so merging them seemed like
> it would be too confusing.

I don't think it has to be.

Maybe something roughly like

- Allow granting the right to perform maintenance operations (Nathan Bossart)

  The permission can be granted on a per-table basis using the MAINTAIN
  privilege and on a system wide basis via the pg_maintain role.

  Operations that can be controlled are VACUUM, ANALYZE, REINDEX, REFRESH
  MATERIALIZED VIEW, CLUSTER, and LOCK TABLE.


I'm again mostly reacting to your concern that the release notes are getting
too boring to read. Repeated content, like in the current formulation, imo
does endanger that. Current it is:

- Add per-table GRANT permission MAINTAIN to control maintenance operations (Nathan Bossart)

  The operations are VACUUM, ANALYZE, REINDEX, REFRESH MATERIALIZED VIEW, CLUSTER, and LOCK TABLE.

- Add user-grantable role pg_maintain to control maintenance operations (Nathan Bossart)

  The operations are VACUUM, ANALYZE, REINDEX, REFRESH MATERIALIZED VIEW, CLUSTER, and LOCK TABLE.



> > - separate entries about pg_stat_reset_slru(), pg_stat_reset_shared("slru"),
>
> They are different functions with different detail text.

So what? You can change their text. Making it three entries makes it harder
for a reader that doesn't care about resetting stats to skip over the details.

Make it something like

- Improve control over resetting statistics (Atsushi Torikoshi, Bharath
  Rupireddy)

  pg_stat_reset_shared() can now reset all shared statistics, by passing NULL;
  pg_stat_reset_shared(NULL) also resets SLRU statistics;
  pg_stat_reset_shared("slru") resets SLRU statistics, which was already
  possible using pg_stat_reset_slru(NULL).

Greetings,

Andres Freund