Date: Thu, 16 May 2024 21:16:46 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: "Imseih (AWS), Sami" <simseih@amazon.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: allow changing autovacuum_max_workers without restarting
Message-ID: <20240517021646.GA1745636@nathanxps13>
References: <20240410212344.GA1824549@nathanxps13>
 <Zh0srZSCSfcK_oY9@pryzbyj2023>
 <20240415162833.GA2857238@nathanxps13>
 <20240415163749.GA2858464@nathanxps13>
 <96B4FC7B-A919-4F93-80A3-BA3EA4F8479A@amazon.com>
 <20240503010415.GA1008894@nathanxps13>
 <2780DB08-DCA6-4E57-B823-303A0E7E17D7@amazon.com>
 <20240507160605.GA2523153@nathanxps13>
 <011CA929-4933-40EA-B1D3-29131FEFBCF0@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <011CA929-4933-40EA-B1D3-29131FEFBCF0@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20240517021646.GA1745636%40nathanxps13>
Precedence: bulk

On Thu, May 16, 2024 at 04:37:10PM +0000, Imseih (AWS), Sami wrote:
> 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.

WFM

> 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

Part of me wonders whether documenting the exact formula is worthwhile.
This portion of the docs is rather complicated, and I can't recall ever
having to do the arithmetic is describes.  Plus, see below...

> 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.

Ha, I think it should actually be "+ 7"!  The value is calculated as

	MaxConnections + autovacuum_max_workers + 1 + max_worker_processes + max_wal_senders + 6

Looking at the history, this documentation tends to be wrong quite often.
In v9.2, the checkpointer was introduced, and these formulas were not
updated.  In v9.3, background worker processes were introduced, and the
formulas were still not updated.  Finally, in v9.6, it was fixed in commit
597f7e3.  Then, in v14, the archiver process was made an auxiliary process
(commit d75288f), making the formulas out-of-date again.  And in v17, the
WAL summarizer was added.

On top of this, IIUC you actually need even more semaphores if your system
doesn't support atomics, and from a quick skim this doesn't seem to be
covered in this documentation.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com