Re: Should vacuum process config file reload more often

Hi, Melanie!

On Fri, 24 Feb 2023 at 02:08, Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> Hi,
>
> Users may wish to speed up long-running vacuum of a large table by
> decreasing autovacuum_vacuum_cost_delay/vacuum_cost_delay, however the
> config file is only reloaded between tables (for autovacuum) or after
> the statement (for explicit vacuum). This has been brought up for
> autovacuum in [1].
>
> Andres suggested that it might be possible to check ConfigReloadPending
> in vacuum_delay_point(), so I thought I would draft a rough patch and
> start a discussion.
>
> Since vacuum_delay_point() is also called by analyze and we do not want
> to reload the configuration file if we are in a user transaction, I
> widened the scope of the in_outer_xact variable in vacuum() and allowed
> analyze in a user transaction to default to the current configuration
> file reload cadence in PostgresMain().
>
> I don't think I can set and leave vac_in_outer_xact the way I am doing
> it in this patch, since I use vac_in_outer_xact in vacuum_delay_point(),
> which I believe is reachable from codepaths that would not have called
> vacuum(). It seems that if a backend sets it, the outer transaction
> commits, and then the backend ends up calling vacuum_delay_point() in a
> different way later, it wouldn't be quite right.
>
> Apart from this, one higher level question I have is if there are other
> gucs whose modification would make reloading the configuration file
> during vacuum/analyze unsafe.

I have a couple of small questions:
Can this patch also read the current GUC value if it's modified by the
SET command, without editing config file?
What will be if we modify config file with mistakes? (When we try to
start the cluster with an erroneous config file it will fail to start,
not sure about re-read config)

Overall the proposal seems legit and useful.

Kind regards,
Pavel Borisov