Re: allow changing autovacuum_max_workers without restarting

From: "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: allow changing autovacuum_max_workers without restarting
Date: 2024-05-16 16:37:10
Message-ID: 011CA929-4933-40EA-B1D3-29131FEFBCF0@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>> That's true, but using a hard-coded limit means we no longer need to add a
>>> new GUC. Always allocating, say, 256 slots might require a few additional
>>> kilobytes of shared memory, most of which will go unused, but that seems
>>> unlikely to be a problem for the systems that will run Postgres v18.
>>
>> I agree with this.

> Here's what this might look like. I chose an upper limit of 1024, which
> seems like it "ought to be enough for anybody," at least for now.

I thought 256 was a good enough limit. In practice, I doubt anyone will
benefit from more than a few dozen autovacuum workers.
I think 1024 is way too high to even allow.

Besides that the overall patch looks good to me, but I have
some comments on the documentation.

I don't think combining 1024 + 5 = 1029 is a good idea in docs.
Breaking down the allotment and using the name of the constant
is much more clear.

I suggest
" max_connections + max_wal_senders + max_worker_processes + AUTOVAC_MAX_WORKER_SLOTS + 5"

and in other places in the docs, we should mention the actual
value of AUTOVAC_MAX_WORKER_SLOTS. Maybe in the
below section?

Instead of:
- (<xref linkend="guc-autovacuum-max-workers"/>) and allowed background
+ (1024) and allowed background

do something like:
- (<xref linkend="guc-autovacuum-max-workers"/>) and allowed background
+ AUTOVAC_MAX_WORKER_SLOTS (1024) and allowed background

Also, replace the 1024 here with AUTOVAC_MAX_WORKER_SLOTS.

+ <varname>max_wal_senders</varname>,
+ plus <varname>max_worker_processes</varname>, plus 1024 for autovacuum
+ worker processes, plus one extra for each 16

Also, Not sure if I am mistaken here, but the "+ 5" in the existing docs
seems wrong.

If it refers to NUM_AUXILIARY_PROCS defined in
include/storage/proc.h, it should a "6"

#define NUM_AUXILIARY_PROCS 6

This is not a consequence of this patch, and can be dealt with
In a separate thread if my understanding is correct.

Regards,

Sami

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-05-16 16:40:50 Re: Docs: Always use true|false instead of sometimes on|off for the subscription options
Previous Message Dmitry Dolgov 2024-05-16 16:25:33 Re: broken JIT support on Fedora 40