Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Give up on running with NetBSD/OpenBSD's default semaphore settings.
- 810a8b1c8051 18.0 landed
initdb: Do not report default autovacuum_worker_slots.
- 2b49492eda3f 18.0 landed
Lower default value of autovacuum_worker_slots in initdb as needed.
- 6d01541960e2 18.0 landed
Allow changing autovacuum_max_workers without restarting.
- c758119e5bfb 18.0 landed
Reserve a PGPROC slot and semaphore for the slotsync worker process.
- 2bdf1b2a2efc 18.0 cited
Make archiver process an auxiliary process.
- d75288fb27b8 14.0 cited
Correct the formulas for System V IPC parameters SEMMNI and SEMMNS in docs.
- 597f7e3a6ec3 9.6.0 cited

allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-10T21:23:44Z

I frequently hear about scenarios where users with thousands upon thousands
of tables realize that autovacuum is struggling to keep up.  When they
inevitably go to bump up autovacuum_max_workers, they discover that it
requires a server restart (i.e., downtime) to take effect, causing further
frustration.  For this reason, I think $SUBJECT is a desirable improvement.
I spent some time looking for past discussions about this, and I was
surprised to not find any, so I thought I'd give it a try.

The attached proof-of-concept patch demonstrates what I have in mind.
Instead of trying to dynamically change the global process table, etc., I'm
proposing that we introduce a new GUC that sets the effective maximum
number of autovacuum workers that can be started at any time.  This means
there would be two GUCs for the number of autovacuum workers: one for the
number of slots reserved for autovacuum workers, and another that restricts
the number of those slots that can be used.  The former would continue to
require a restart to change its value, and users would typically want to
set it relatively high.  The latter could be changed at any time and would
allow for raising or lowering the maximum number of active autovacuum
workers, up to the limit set by the other parameter.

The proof-of-concept patch keeps autovacuum_max_workers as the maximum
number of slots to reserve for workers, but I think we should instead
rename this parameter to something else and then reintroduce
autovacuum_max_workers as the new parameter that can be adjusted without
restarting.  That way, autovacuum_max_workers continues to work much the
same way as in previous versions.

There are a couple of weird cases with this approach.  One is when the
restart-only limit is set lower than the PGC_SIGHUP limit.  In that case, I
think we should just use the restart-only limit.  The other is when there
are already N active autovacuum workers and the PGC_SIGHUP parameter is
changed to something less than N.  For that case, I think we should just
block starting additional workers until the number of workers drops below
the new parameter's value.  I don't think we should kill existing workers,
or anything else like that.

TBH I've been sitting on this idea for a while now, only because I think it
has a slim chance of acceptance, but IMHO this is a simple change that
could help many users.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-11T14:24:18Z

> I frequently hear about scenarios where users with thousands upon thousands
> of tables realize that autovacuum is struggling to keep up.  When they
> inevitably go to bump up autovacuum_max_workers, they discover that it
> requires a server restart (i.e., downtime) to take effect, causing further
> frustration.  For this reason, I think $SUBJECT is a desirable improvement.
> I spent some time looking for past discussions about this, and I was
> surprised to not find any, so I thought I'd give it a try.

I did not review the patch in detail yet, but +1 to the idea. 
It's not just thousands of tables that suffer from this.
If a user has a few large tables hogging the autovac workers, then other
tables don't get the autovac cycles they require. Users are then forced
to run manual vacuums, which adds complexity to their operations.

> The attached proof-of-concept patch demonstrates what I have in mind.
> Instead of trying to dynamically change the global process table, etc., I'm
> proposing that we introduce a new GUC that sets the effective maximum
> number of autovacuum workers that can be started at any time.

max_worker_processes defines a pool of max # of background workers allowed.
parallel workers and extensions that spin  up background workers all utilize from 
this pool. 

Should autovacuum_max_workers be able to utilize from max_worker_processes also?

This will allow autovacuum_max_workers to be dynamic while the user only has
to deal with an already existing GUC. We may want to increase the default value
for max_worker_processes as part of this.


Regards,

Sami
Amazon Web Services (AWS)

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-11T14:42:40Z

On Thu, Apr 11, 2024 at 02:24:18PM +0000, Imseih (AWS), Sami wrote:
> max_worker_processes defines a pool of max # of background workers allowed.
> parallel workers and extensions that spin  up background workers all utilize from 
> this pool. 
> 
> Should autovacuum_max_workers be able to utilize from max_worker_processes also?
> 
> This will allow autovacuum_max_workers to be dynamic while the user only has
> to deal with an already existing GUC. We may want to increase the default value
> for max_worker_processes as part of this.

My concern with this approach is that other background workers could use up
all the slots and prevent autovacuum workers from starting, unless of
course we reserve autovacuum_max_workers slots for _only_ autovacuum
workers.  I'm not sure if we want to get these parameters tangled up like
this, though...

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

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-11T14:58:34Z

On Thu, Apr 11, 2024 at 09:42:40AM -0500, Nathan Bossart wrote:
> On Thu, Apr 11, 2024 at 02:24:18PM +0000, Imseih (AWS), Sami wrote:
>> max_worker_processes defines a pool of max # of background workers allowed.
>> parallel workers and extensions that spin  up background workers all utilize from 
>> this pool. 
>> 
>> Should autovacuum_max_workers be able to utilize from max_worker_processes also?
>> 
>> This will allow autovacuum_max_workers to be dynamic while the user only has
>> to deal with an already existing GUC. We may want to increase the default value
>> for max_worker_processes as part of this.
> 
> My concern with this approach is that other background workers could use up
> all the slots and prevent autovacuum workers from starting, unless of
> course we reserve autovacuum_max_workers slots for _only_ autovacuum
> workers.  I'm not sure if we want to get these parameters tangled up like
> this, though...

I see that the logical replication launcher process uses this pool, but we
take special care to make sure it gets a slot:

	/*
	 * Register the apply launcher.  It's probably a good idea to call this
	 * before any modules had a chance to take the background worker slots.
	 */
	ApplyLauncherRegister();

I'm not sure there's another way to effectively reserve slots that would
work for the autovacuum workers (which need to restart to connect to
different databases), so that would need to be invented.  We'd probably
also want to fail startup if autovacuum_max_workers < max_worker_processes,
which seems like it has the potential to cause problems when folks first
upgrade to v18.

Furthermore, we might have to convert autovacuum workers to background
worker processes for this to work.  I've admittedly wondered about whether
we should do that eventually, anyway, but it'd expand the scope of this
work quite a bit.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-11T15:37:23Z

> My concern with this approach is that other background workers could use up
> all the slots and prevent autovacuum workers from starting

That's a good point, the current settings do not guarantee that you
get a worker for the purpose if none are available, 
i.e. max_parallel_workers_per_gather, you may have 2 workers planned 
and 0 launched. 

> unless of
> course we reserve autovacuum_max_workers slots for _only_ autovacuum
> workers. I'm not sure if we want to get these parameters tangled up like
> this, though...

This will be confusing to describe and we will be reserving autovac workers
implicitly, rather than explicitly with a new GUC.

Regards,

Sami

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-11T19:24:23Z

On Thu, Apr 11, 2024 at 03:37:23PM +0000, Imseih (AWS), Sami wrote:
>> My concern with this approach is that other background workers could use up
>> all the slots and prevent autovacuum workers from starting
> 
> That's a good point, the current settings do not guarantee that you
> get a worker for the purpose if none are available, 
> i.e. max_parallel_workers_per_gather, you may have 2 workers planned 
> and 0 launched. 
> 
>> unless of
>> course we reserve autovacuum_max_workers slots for _only_ autovacuum
>> workers. I'm not sure if we want to get these parameters tangled up like
>> this, though...
> 
> This will be confusing to describe and we will be reserving autovac workers
> implicitly, rather than explicitly with a new GUC.

Yeah, that's probably a good reason to give autovacuum its own worker pool.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-12T17:27:40Z

I spent sometime reviewing/testing the POC. It is relatively simple with a lot
of obvious value. 

I tested with 16 tables that constantly reach the autovac threashold and the
patch did the right thing. I observed concurrent autovacuum workers matching
the setting as I was adjusting it dynamically.

As you mention above, If there are more autovacs in progress and a new lower setting 
is applied, we should not take any special action on those autovacuums, and eventually 
the max number of autovacuum workers will match the setting.

I also tested by allowing user connections to reach max_connections, and observed the 
expected number of autovacuums spinning up and correctly adjusted.

Having autovacuum tests ( unless I missed something ) like the above is a good 
general improvement, but it should not be tied to this. 

A few comments on  the POC patch:

1/ We should emit a log when autovacuum_workers is set higher than the max.

2/ should the name of the restart limit be "reserved_autovacuum_workers"?

Regards,

Sami Imseih
AWS (Amazon Web Services)

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-12T18:40:00Z

On Fri, Apr 12, 2024 at 05:27:40PM +0000, Imseih (AWS), Sami wrote:
> A few comments on  the POC patch:

Thanks for reviewing.

> 1/ We should emit a log when autovacuum_workers is set higher than the max.

Hm.  Maybe the autovacuum launcher could do that.

> 2/ should the name of the restart limit be "reserved_autovacuum_workers"?

That's kind-of what I had in mind, although I think we might want to avoid
the word "reserved" because it sounds a bit like reserved_connections and
superuser_reserved_connections.  "autovacuum_max_slots" or
"autovacuum_max_worker_slots" might be worth considering, too.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-12T22:17:44Z

>> 1/ We should emit a log when autovacuum_workers is set higher than the max.


>> Hm. Maybe the autovacuum launcher could do that.

Would it be better to use a GUC check_hook that compares the 
new value with the max allowed values and emits a WARNING ?

autovacuum_max_workers already has a check_autovacuum_max_workers
check_hook, which can be repurposed for this.

In the POC patch, this check_hook is kept as-is, which will no longer make sense.

>> 2/ should the name of the restart limit be "reserved_autovacuum_workers"?


>> That's kind-of what I had in mind, although I think we might want to avoid
>> the word "reserved" because it sounds a bit like reserved_connections 
>> and superuser_reserved_connections

Yes, I agree. This can be confusing.

>>  "autovacuum_max_slots" or
>> "autovacuum_max_worker_slots" might be worth considering, too.

"autovacuum_max_worker_slots" is probably the best option because
we should have "worker" in the name of the GUC.


Regards,

Sami

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-13T19:44:50Z

On Fri, Apr 12, 2024 at 10:17:44PM +0000, Imseih (AWS), Sami wrote:
>>> Hm. Maybe the autovacuum launcher could do that.
> 
> Would it be better to use a GUC check_hook that compares the 
> new value with the max allowed values and emits a WARNING ?
> 
> autovacuum_max_workers already has a check_autovacuum_max_workers
> check_hook, which can be repurposed for this.
> 
> In the POC patch, this check_hook is kept as-is, which will no longer make sense.

IIRC using GUC hooks to handle dependencies like this is generally frowned
upon because it tends to not work very well [0].  We could probably get it
to work for this particular case, but IMHO we should still try to avoid
this approach.  I didn't find any similar warnings for other GUCs like
max_parallel_workers_per_gather, so it might not be crucial to emit a
WARNING here.

[0] https://postgr.es/m/27574.1581015893%40sss.pgh.pa.us

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-14T01:56:09Z

> IIRC using GUC hooks to handle dependencies like this is generally frowned
> upon because it tends to not work very well [0]. We could probably get it
> to work for this particular case, but IMHO we should still try to avoid
> this approach. 

Thanks for pointing this out. I agree, this could lead to false logs being
emitted.

> so it might not be crucial to emit a
> WARNING here.

As mentioned earlier in the thread, we can let the autovacuum launcher emit the 
log,  but it will need to be careful not flood the logs when this condition exists ( i.e.
log only the first time the condition is detected or log every once in a while )

The additional complexity is not worth it.

Regards,

Sami

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-14T14:40:58Z

Here is a first attempt at a proper patch set based on the discussion thus
far.  I've split it up into several small patches for ease of review, which
is probably a bit excessive.  If this ever makes it to commit, they could
likely be combined.

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

Re: allow changing autovacuum_max_workers without restarting

Justin Pryzby <pryzby@telsasoft.com> — 2024-04-15T13:33:33Z

On Wed, Apr 10, 2024 at 04:23:44PM -0500, Nathan Bossart wrote:
> The attached proof-of-concept patch demonstrates what I have in mind.
> Instead of trying to dynamically change the global process table, etc., I'm
> proposing that we introduce a new GUC that sets the effective maximum
> number of autovacuum workers that can be started at any time.  This means
> there would be two GUCs for the number of autovacuum workers: one for the
> number of slots reserved for autovacuum workers, and another that restricts
> the number of those slots that can be used.  The former would continue to
> require a restart to change its value, and users would typically want to
> set it relatively high.  The latter could be changed at any time and would
> allow for raising or lowering the maximum number of active autovacuum
> workers, up to the limit set by the other parameter.
> 
> The proof-of-concept patch keeps autovacuum_max_workers as the maximum
> number of slots to reserve for workers, but I think we should instead
> rename this parameter to something else and then reintroduce
> autovacuum_max_workers as the new parameter that can be adjusted without
> restarting.  That way, autovacuum_max_workers continues to work much the
> same way as in previous versions.

When I thought about this, I considered proposing to add a new GUC for
"autovacuum_policy_workers".

autovacuum_max_workers would be the same as before, requiring a restart
to change.  The policy GUC would be the soft limit, changable at runtime
up to the hard limit of autovacuum_max_workers (or maybe any policy
value exceeding autovacuum_max_workers would be ignored).

We'd probably change autovacuum_max_workers to default to a higher value
(8, or 32 as in your patch), and have autovacuum_max_workers default to
3, for consistency with historic behavior.  Maybe
autovacuum_policy_workers=-1 would mean to use all workers.

There's the existing idea to change autovacuum thresholds during the
busy period of the day vs. off hours.  This would allow something
similar with nworkers rather than thresholds: if the goal were to reduce
the resource use of vacuum, the admin could set max_workers=8, with
policy_workers=2 during the busy period.

-- 
Justin

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-15T16:28:33Z

On Mon, Apr 15, 2024 at 08:33:33AM -0500, Justin Pryzby wrote:
> On Wed, Apr 10, 2024 at 04:23:44PM -0500, Nathan Bossart wrote:
>> The proof-of-concept patch keeps autovacuum_max_workers as the maximum
>> number of slots to reserve for workers, but I think we should instead
>> rename this parameter to something else and then reintroduce
>> autovacuum_max_workers as the new parameter that can be adjusted without
>> restarting.  That way, autovacuum_max_workers continues to work much the
>> same way as in previous versions.
> 
> When I thought about this, I considered proposing to add a new GUC for
> "autovacuum_policy_workers".
> 
> autovacuum_max_workers would be the same as before, requiring a restart
> to change.  The policy GUC would be the soft limit, changable at runtime
> up to the hard limit of autovacuum_max_workers (or maybe any policy
> value exceeding autovacuum_max_workers would be ignored).
> 
> We'd probably change autovacuum_max_workers to default to a higher value
> (8, or 32 as in your patch), and have autovacuum_max_workers default to
> 3, for consistency with historic behavior.  Maybe
> autovacuum_policy_workers=-1 would mean to use all workers.

This sounds like roughly the same idea, although it is backwards from what
I'm proposing in the v1 patch set.  My thinking is that by making a new
restart-only GUC that would by default be set higher than the vast majority
of systems should ever need, we could simplify migrating to these
parameters.  The autovacuum_max_workers parameter would effectively retain
it's original meaning, and existing settings would continue to work
normally on v18, but users could now adjust it without restarting.  If we
did it the other way, users would need to bump up autovacuum_max_workers
and restart prior to being able to raise autovacuum_policy_workers beyond
what they previously had set for autovacuum_max_workers.  That being said,
I'm open to doing it this way if folks prefer this approach, as I think it
is still an improvement.

> There's the existing idea to change autovacuum thresholds during the
> busy period of the day vs. off hours.  This would allow something
> similar with nworkers rather than thresholds: if the goal were to reduce
> the resource use of vacuum, the admin could set max_workers=8, with
> policy_workers=2 during the busy period.

Precisely.

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

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-15T16:37:49Z

On Mon, Apr 15, 2024 at 11:28:33AM -0500, Nathan Bossart wrote:
> On Mon, Apr 15, 2024 at 08:33:33AM -0500, Justin Pryzby wrote:
>> On Wed, Apr 10, 2024 at 04:23:44PM -0500, Nathan Bossart wrote:
>>> The proof-of-concept patch keeps autovacuum_max_workers as the maximum
>>> number of slots to reserve for workers, but I think we should instead
>>> rename this parameter to something else and then reintroduce
>>> autovacuum_max_workers as the new parameter that can be adjusted without
>>> restarting.  That way, autovacuum_max_workers continues to work much the
>>> same way as in previous versions.
>> 
>> When I thought about this, I considered proposing to add a new GUC for
>> "autovacuum_policy_workers".
>> 
>> autovacuum_max_workers would be the same as before, requiring a restart
>> to change.  The policy GUC would be the soft limit, changable at runtime
>> up to the hard limit of autovacuum_max_workers (or maybe any policy
>> value exceeding autovacuum_max_workers would be ignored).
>> 
>> We'd probably change autovacuum_max_workers to default to a higher value
>> (8, or 32 as in your patch), and have autovacuum_max_workers default to
>> 3, for consistency with historic behavior.  Maybe
>> autovacuum_policy_workers=-1 would mean to use all workers.
> 
> This sounds like roughly the same idea, although it is backwards from what
> I'm proposing in the v1 patch set.  My thinking is that by making a new
> restart-only GUC that would by default be set higher than the vast majority
> of systems should ever need, we could simplify migrating to these
> parameters.  The autovacuum_max_workers parameter would effectively retain
> it's original meaning, and existing settings would continue to work
> normally on v18, but users could now adjust it without restarting.  If we
> did it the other way, users would need to bump up autovacuum_max_workers
> and restart prior to being able to raise autovacuum_policy_workers beyond
> what they previously had set for autovacuum_max_workers.  That being said,
> I'm open to doing it this way if folks prefer this approach, as I think it
> is still an improvement.

Another option could be to just remove the restart-only GUC and hard-code
the upper limit of autovacuum_max_workers to 64 or 128 or something.  While
that would simplify matters, I suspect it would be hard to choose an
appropriate limit that won't quickly become outdated.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-15T17:41:04Z

> Another option could be to just remove the restart-only GUC and hard-code
> the upper limit of autovacuum_max_workers to 64 or 128 or something. While
> that would simplify matters, I suspect it would be hard to choose an
> appropriate limit that won't quickly become outdated.

Hardcoded values are usually hard to deal with because they are hidden either
In code or in docs.

> When I thought about this, I considered proposing to add a new GUC for
> "autovacuum_policy_workers".

> autovacuum_max_workers would be the same as before, requiring a restart
> to change.  The policy GUC would be the soft limit, changable at runtime

I think autovacuum_max_workers should still be the GUC that controls
the number of concurrent autovacuums. This parameter is already well 
established and changing the meaning now will be confusing. 

I suspect most users will be glad it's now dynamic, but will probably 
be annoyed if it's no longer doing what it's supposed to.

Regards,

Sami

Re: allow changing autovacuum_max_workers without restarting

wenhui qiu <qiuwenhuifx@gmail.com> — 2024-04-17T06:52:16Z

Agree +1,From a dba perspective, I would prefer that this parameter can be
dynamically modified, rather than adding a new parameter,What is more
difficult is how to smoothly reach the target value when the setting is
considered to be too large and needs to be lowered.



Regards

On Tue, 16 Apr 2024 at 01:41, Imseih (AWS), Sami <simseih@amazon.com> wrote:

> > Another option could be to just remove the restart-only GUC and hard-code
> > the upper limit of autovacuum_max_workers to 64 or 128 or something.
> While
> > that would simplify matters, I suspect it would be hard to choose an
> > appropriate limit that won't quickly become outdated.
>
> Hardcoded values are usually hard to deal with because they are hidden
> either
> In code or in docs.
>
> > When I thought about this, I considered proposing to add a new GUC for
> > "autovacuum_policy_workers".
>
> > autovacuum_max_workers would be the same as before, requiring a restart
> > to change.  The policy GUC would be the soft limit, changable at runtime
>
> I think autovacuum_max_workers should still be the GUC that controls
> the number of concurrent autovacuums. This parameter is already well
> established and changing the meaning now will be confusing.
>
> I suspect most users will be glad it's now dynamic, but will probably
> be annoyed if it's no longer doing what it's supposed to.
>
> Regards,
>
> Sami
>
>

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-18T05:05:03Z

> Here is a first attempt at a proper patch set based on the discussion thus
> far. I've split it up into several small patches for ease of review, which
> is probably a bit excessive. If this ever makes it to commit, they could
> likely be combined.

I looked at the patch set. With the help of DEBUG2 output, I tested to ensure
that the the autovacuum_cost_limit  balance adjusts correctly when the 
autovacuum_max_workers value increases/decreases. I did not think the 
patch will break this behavior, but it's important to verify this.

Some comments on the patch:

1. A nit. There should be a tab here.

-       dlist_head      av_freeWorkers;
+       dclist_head av_freeWorkers;

2. autovacuum_max_worker_slots documentation:

+       <para>
+        Note that the value of <xref linkend="guc-autovacuum-max-workers"/> is
+        silently capped to this value.
+       </para>

This comment looks redundant in the docs, since the entry 
for autovacuum_max_workers that follows mentions the
same.


3. The docs for autovacuum_max_workers should mention that when
the value changes, consider adjusting the autovacuum_cost_limit/cost_delay. 

This is not something new. Even in the current state, users should think about 
these settings. However, it seems even important if this value is to be 
dynamically adjusted.


Regards,

Sami

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-19T15:43:22Z

On Thu, Apr 18, 2024 at 05:05:03AM +0000, Imseih (AWS), Sami wrote:
> I looked at the patch set. With the help of DEBUG2 output, I tested to ensure
> that the the autovacuum_cost_limit  balance adjusts correctly when the 
> autovacuum_max_workers value increases/decreases. I did not think the 
> patch will break this behavior, but it's important to verify this.

Great.

> 1. A nit. There should be a tab here.
> 
> -       dlist_head      av_freeWorkers;
> +       dclist_head av_freeWorkers;

I dare not argue with pgindent.

> 2. autovacuum_max_worker_slots documentation:
> 
> +       <para>
> +        Note that the value of <xref linkend="guc-autovacuum-max-workers"/> is
> +        silently capped to this value.
> +       </para>
> 
> This comment looks redundant in the docs, since the entry 
> for autovacuum_max_workers that follows mentions the
> same.

Removed in v2.  I also noticed that I forgot to update the part about when
autovacuum_max_workers can be changed.  *facepalm*

> 3. The docs for autovacuum_max_workers should mention that when
> the value changes, consider adjusting the autovacuum_cost_limit/cost_delay. 
> 
> This is not something new. Even in the current state, users should think about 
> these settings. However, it seems even important if this value is to be 
> dynamically adjusted.

I don't necessarily disagree that it might be worth mentioning these
parameters, but I would argue that this should be proposed in a separate
thread.

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

Re: allow changing autovacuum_max_workers without restarting

Robert Haas <robertmhaas@gmail.com> — 2024-04-19T18:42:13Z

On Fri, Apr 19, 2024 at 11:43 AM Nathan Bossart
<nathandbossart@gmail.com> wrote:
> Removed in v2.  I also noticed that I forgot to update the part about when
> autovacuum_max_workers can be changed.  *facepalm*

I think this could help a bunch of users, but I'd still like to
complain, not so much with the desire to kill this patch as with the
desire to broaden the conversation.

Part of the underlying problem here is that, AFAIK, neither PostgreSQL
as a piece of software nor we as human beings who operate PostgreSQL
databases have much understanding of how autovacuum_max_workers should
be set. It's relatively easy to hose yourself by raising
autovacuum_max_workers to try to make things go faster, but produce
the exact opposite effect due to how the cost balancing stuff works.
But, even if you have the correct use case for autovacuum_max_workers,
something like a few large tables that take a long time to vacuum plus
a bunch of smaller ones that can't get starved just because the big
tables are in the midst of being processed, you might well ask
yourself why it's your job to figure out the correct number of
workers.

Now, before this patch, there is a fairly good reason for that, which
is that we need to reserve shared memory resources for each autovacuum
worker that might potentially run, and the system can't know how much
shared memory you'd like to reserve for that purpose. But if that were
the only problem, then this patch would probably just be proposing to
crank up the default value of that parameter rather than introducing a
second one. I bet Nathan isn't proposing that because his intuition is
that it will work out badly, and I think he's right. I bet that
cranking up the number of allowed workers will often result in running
more workers than we really should. One possible negative consequence
is that we'll end up with multiple processes fighting over the disk in
a situation where they should just take turns. I suspect there are
also ways that we can be harmed - in broadly similar fashion - by cost
balancing.

So I feel like what this proposal reveals is that we know that our
algorithm for ramping up the number of running workers doesn't really
work. And maybe that's just a consequence of the general problem that
we have no global information about how much vacuuming work there is
to be done at any given time, and therefore we cannot take any kind of
sensible guess about whether 1 more worker will help or hurt. Or,
maybe there's some way to do better than what we do today without a
big rewrite. I'm not sure. I don't think this patch should be burdened
with solving the general problem here. But I do think the general
problem is worth some discussion.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-04-19T20:29:31Z

On Fri, Apr 19, 2024 at 02:42:13PM -0400, Robert Haas wrote:
> I think this could help a bunch of users, but I'd still like to
> complain, not so much with the desire to kill this patch as with the
> desire to broaden the conversation.

I think I subconsciously hoped this would spark a bigger discussion...

> Now, before this patch, there is a fairly good reason for that, which
> is that we need to reserve shared memory resources for each autovacuum
> worker that might potentially run, and the system can't know how much
> shared memory you'd like to reserve for that purpose. But if that were
> the only problem, then this patch would probably just be proposing to
> crank up the default value of that parameter rather than introducing a
> second one. I bet Nathan isn't proposing that because his intuition is
> that it will work out badly, and I think he's right. I bet that
> cranking up the number of allowed workers will often result in running
> more workers than we really should. One possible negative consequence
> is that we'll end up with multiple processes fighting over the disk in
> a situation where they should just take turns. I suspect there are
> also ways that we can be harmed - in broadly similar fashion - by cost
> balancing.

Even if we were content to bump up the default value of
autovacuum_max_workers and tell folks to just mess with the cost settings,
there are still probably many cases where bumping up the number of workers
further would be necessary.  If you have a zillion tables, turning
cost-based vacuuming off completely may be insufficient to keep up, at
which point your options become limited.  It can be difficult to tell
whether you might end up in this situation over time as your workload
evolves.  In any case, it's not clear to me that bumping up the default
value of autovacuum_max_workers would do more good than harm.  I get the
idea that the default of 3 is sufficient for a lot of clusters, so there'd
really be little upside to changing it AFAICT.  (I guess this proves your
point about my intuition.)

> So I feel like what this proposal reveals is that we know that our
> algorithm for ramping up the number of running workers doesn't really
> work. And maybe that's just a consequence of the general problem that
> we have no global information about how much vacuuming work there is
> to be done at any given time, and therefore we cannot take any kind of
> sensible guess about whether 1 more worker will help or hurt. Or,
> maybe there's some way to do better than what we do today without a
> big rewrite. I'm not sure. I don't think this patch should be burdened
> with solving the general problem here. But I do think the general
> problem is worth some discussion.

I certainly don't want to hold up $SUBJECT for a larger rewrite of
autovacuum scheduling, but I also don't want to shy away from a larger
rewrite if it's an idea whose time has come.  I'm looking forward to
hearing your ideas in your pgconf.dev talk.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-04-21T23:26:59Z

> Part of the underlying problem here is that, AFAIK, neither PostgreSQL
> as a piece of software nor we as human beings who operate PostgreSQL
> databases have much understanding of how autovacuum_max_workers should
> be set. It's relatively easy to hose yourself by raising
> autovacuum_max_workers to try to make things go faster, but produce
> the exact opposite effect due to how the cost balancing stuff works.

Yeah, this patch will not fix this problem. Anyone who raises av_max_workers
should think about adjusting the av_cost_delay. This was discussed up the
thread [4] and even without this patch, I think it's necessary to add more
documentation on the relationship between workers and cost.


> So I feel like what this proposal reveals is that we know that our
> algorithm for ramping up the number of running workers doesn't really
> work. And maybe that's just a consequence of the general problem that
> we have no global information about how much vacuuming work there is
> to be done at any given time, and therefore we cannot take any kind of
> sensible guess about whether 1 more worker will help or hurt. Or,
> maybe there's some way to do better than what we do today without a
> big rewrite. I'm not sure. I don't think this patch should be burdened
> with solving the general problem here. But I do think the general
> problem is worth some discussion.

This patch is only solving the operational problem of adjusting 
autovacuum_max_workers,  and it does so without introducing complexity.

A proposal that will alleviate the users from the burden of having to think about
autovacuum_max_workers, cost_delay and cost_limit settings will be great.
This patch may be the basis for such dynamic  "auto-tuning" of autovacuum workers.


Regards,

Sami

[4] https://www.postgresql.org/message-id/20240419154322.GA3988554%40nathanxps13

Re: allow changing autovacuum_max_workers without restarting

Robert Haas <robertmhaas@gmail.com> — 2024-04-22T14:17:19Z

On Fri, Apr 19, 2024 at 4:29 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> I certainly don't want to hold up $SUBJECT for a larger rewrite of
> autovacuum scheduling, but I also don't want to shy away from a larger
> rewrite if it's an idea whose time has come.  I'm looking forward to
> hearing your ideas in your pgconf.dev talk.

Yeah, I suppose I was hoping you were going to tell me the all the
answers and thus make the talk a lot easier to write, but I guess life
isn't that simple. :-)

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-05-03T01:04:15Z

On Mon, Apr 15, 2024 at 05:41:04PM +0000, Imseih (AWS), Sami wrote:
>> Another option could be to just remove the restart-only GUC and hard-code
>> the upper limit of autovacuum_max_workers to 64 or 128 or something. While
>> that would simplify matters, I suspect it would be hard to choose an
>> appropriate limit that won't quickly become outdated.
> 
> Hardcoded values are usually hard to deal with because they are hidden either
> In code or in docs.

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.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-05-03T12:57:18Z

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


Regards,

Sami

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-05-07T16:06:05Z

On Fri, May 03, 2024 at 12:57:18PM +0000, Imseih (AWS), Sami wrote:
>> 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.

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

Re: allow changing autovacuum_max_workers without restarting

Sami Imseih <simseih@amazon.com> — 2024-05-16T16:37:10Z

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

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-05-17T02:16:46Z

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

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-03T18:52:29Z

On Thu, May 16, 2024 at 09:16:46PM -0500, Nathan Bossart wrote:
> 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

Here is an updated patch that uses 256 as the upper limit.

>> 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 plan to further improve this section of the documentation in v18, so I've
left the constant unexplained for now.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Andres Freund <andres@anarazel.de> — 2024-06-03T19:08:52Z

Hi,

On 2024-06-03 13:52:29 -0500, Nathan Bossart wrote:
> Here is an updated patch that uses 256 as the upper limit.

I don't have time to read through the entire thread right now - it'd be good
for the commit message of a patch like this to include justification for why
it's ok to make such a change. Even before actually committing it, so
reviewers have an easier time catching up.

Why do we think that increasing the number of PGPROC slots, heavyweight locks
etc by 256 isn't going to cause issues?  That's not an insubstantial amount of
memory to dedicate to something that will practically never be used.

ISTM that at the very least we ought to exclude the reserved slots from the
computation of things like the number of locks resulting from
max_locks_per_transaction.  It's very common to increase
max_locks_per_transaction substantially, adding ~250 to the multiplier can be
a good amount of memory. And AV workers should never need a meaningful number.

Increasing e.g. the size of the heavyweight lock table has consequences
besides the increase in memory usage, the size increase can make it less
likely for the table to fit largely into L3, thus decreasing performance.

Greetings,

Andres Freund

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-03T19:28:13Z

On Mon, Jun 03, 2024 at 12:08:52PM -0700, Andres Freund wrote:
> I don't have time to read through the entire thread right now - it'd be good
> for the commit message of a patch like this to include justification for why
> it's ok to make such a change. Even before actually committing it, so
> reviewers have an easier time catching up.

Sorry about that.  I think the main question (besides "should we do this?")
is whether we ought to make the upper limit configurable.  My initial idea
was to split autovacuum_max_workers into two GUCs: one for the upper limit
that only be changed at server start and another for the effective limit
that can be changed up to the upper limit without restarting the server.
If we can just set a sufficiently high upper limit and avoid the extra GUC
without causing problems, that might be preferable, but I sense that you
are about to tell me that it will indeed cause problems.  :)

> Why do we think that increasing the number of PGPROC slots, heavyweight locks
> etc by 256 isn't going to cause issues?  That's not an insubstantial amount of
> memory to dedicate to something that will practically never be used.

I personally have not observed problems with these kinds of bumps in
resource usage, although I may be biased towards larger systems where it
doesn't matter as much.

> ISTM that at the very least we ought to exclude the reserved slots from the
> computation of things like the number of locks resulting from
> max_locks_per_transaction.  It's very common to increase
> max_locks_per_transaction substantially, adding ~250 to the multiplier can be
> a good amount of memory. And AV workers should never need a meaningful number.

This is an interesting idea.

> Increasing e.g. the size of the heavyweight lock table has consequences
> besides the increase in memory usage, the size increase can make it less
> likely for the table to fit largely into L3, thus decreasing performance.

IMHO this might be a good argument for making the upper limit configurable
and setting it relatively low by default.  That's not quite as nice from a
user experience perspective, but weird, hard-to-diagnose performance issues
are certainly not nice, either.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Andres Freund <andres@anarazel.de> — 2024-06-03T23:24:27Z

Hi,

On 2024-06-03 14:28:13 -0500, Nathan Bossart wrote:
> On Mon, Jun 03, 2024 at 12:08:52PM -0700, Andres Freund wrote:
> > Why do we think that increasing the number of PGPROC slots, heavyweight locks
> > etc by 256 isn't going to cause issues?  That's not an insubstantial amount of
> > memory to dedicate to something that will practically never be used.
> 
> I personally have not observed problems with these kinds of bumps in
> resource usage, although I may be biased towards larger systems where it
> doesn't matter as much.

IME it matters *more* on larger systems. Or at least used to, I haven't
experimented with this in quite a while.

It's possible that we improved a bunch of things sufficiently for this to not
matter anymore.

Greetings,

Andres Freund

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-18T19:00:00Z

On Mon, Jun 03, 2024 at 04:24:27PM -0700, Andres Freund wrote:
> On 2024-06-03 14:28:13 -0500, Nathan Bossart wrote:
>> On Mon, Jun 03, 2024 at 12:08:52PM -0700, Andres Freund wrote:
>> > Why do we think that increasing the number of PGPROC slots, heavyweight locks
>> > etc by 256 isn't going to cause issues?  That's not an insubstantial amount of
>> > memory to dedicate to something that will practically never be used.
>> 
>> I personally have not observed problems with these kinds of bumps in
>> resource usage, although I may be biased towards larger systems where it
>> doesn't matter as much.
> 
> IME it matters *more* on larger systems. Or at least used to, I haven't
> experimented with this in quite a while.
> 
> It's possible that we improved a bunch of things sufficiently for this to not
> matter anymore.

I'm curious if there is something specific you would look into to verify
this.  IIUC one concern is the lock table not fitting into L3.  Is there
anything else?  Any particular workloads you have in mind?

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Andres Freund <andres@anarazel.de> — 2024-06-18T20:43:34Z

Hi,

On 2024-06-18 14:00:00 -0500, Nathan Bossart wrote:
> On Mon, Jun 03, 2024 at 04:24:27PM -0700, Andres Freund wrote:
> > On 2024-06-03 14:28:13 -0500, Nathan Bossart wrote:
> >> On Mon, Jun 03, 2024 at 12:08:52PM -0700, Andres Freund wrote:
> >> > Why do we think that increasing the number of PGPROC slots, heavyweight locks
> >> > etc by 256 isn't going to cause issues?  That's not an insubstantial amount of
> >> > memory to dedicate to something that will practically never be used.
> >> 
> >> I personally have not observed problems with these kinds of bumps in
> >> resource usage, although I may be biased towards larger systems where it
> >> doesn't matter as much.
> > 
> > IME it matters *more* on larger systems. Or at least used to, I haven't
> > experimented with this in quite a while.
> > 
> > It's possible that we improved a bunch of things sufficiently for this to not
> > matter anymore.
> 
> I'm curious if there is something specific you would look into to verify
> this.  IIUC one concern is the lock table not fitting into L3.  Is there
> anything else?  Any particular workloads you have in mind?

That was the main thing I was thinking of.


But I think I just thought of one more: It's going to *substantially* increase
the resource usage for tap tests.  Right now Cluster.pm has
		# conservative settings to ensure we can run multiple postmasters:
		print $conf "shared_buffers = 1MB\n";
		print $conf "max_connections = 10\n";

for nodes that allow streaming.

Adding 256 extra backend slots increases the shared memory usage from ~5MB to
~18MB.


I just don't see much point in reserving 256 worker "possibilities", tbh. I
can't think of any practical system where it makes sense to use this much (nor
do I think it's going to be reasonable in the next 10 years) and it's just
going to waste memory and startup time for everyone.

Nor does it make sense to me to have the max autovac workers be independent of
max_connections.

Greetings,

Andres Freund

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-18T21:09:09Z

On Tue, Jun 18, 2024 at 01:43:34PM -0700, Andres Freund wrote:
> I just don't see much point in reserving 256 worker "possibilities", tbh. I
> can't think of any practical system where it makes sense to use this much (nor
> do I think it's going to be reasonable in the next 10 years) and it's just
> going to waste memory and startup time for everyone.

Given this, here are some options I see for moving this forward:

* lower the cap to, say, 64 or 32
* exclude autovacuum worker slots from computing number of locks, etc.
* make the cap configurable and default it to something low (e.g., 8)

My intent with a reserved set of 256 slots was to prevent users from
needing to deal with two GUCs.  For all practical purposes, it would be
possible to change autovacuum_max_workers whenever you want.  But if the
extra resource requirements are too much of a tax, I'm content to change
course.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Andres Freund <andres@anarazel.de> — 2024-06-18T21:33:31Z

Hi,

On 2024-06-18 16:09:09 -0500, Nathan Bossart wrote:
> On Tue, Jun 18, 2024 at 01:43:34PM -0700, Andres Freund wrote:
> > I just don't see much point in reserving 256 worker "possibilities", tbh. I
> > can't think of any practical system where it makes sense to use this much (nor
> > do I think it's going to be reasonable in the next 10 years) and it's just
> > going to waste memory and startup time for everyone.
> 
> Given this, here are some options I see for moving this forward:
> 
> * lower the cap to, say, 64 or 32
> * exclude autovacuum worker slots from computing number of locks, etc.

That seems good regardless

> * make the cap configurable and default it to something low (e.g., 8)


Another one:

Have a general cap of 64, but additionally limit it to something like
     max(1, min(WORKER_CAP, max_connections / 4))

so that cases like tap tests don't end up allocating vastly more worker slots
than actual connection slots.


> My intent with a reserved set of 256 slots was to prevent users from
> needing to deal with two GUCs.  For all practical purposes, it would be
> possible to change autovacuum_max_workers whenever you want.  But if the
> extra resource requirements are too much of a tax, I'm content to change
> course.

Approximately tripling shared memory usage for tap test instances does seem
too much to me.

Greetings,

Andres Freund

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-19T00:43:36Z

On Tue, Jun 18, 2024 at 02:33:31PM -0700, Andres Freund wrote:
> Another one:
> 
> Have a general cap of 64, but additionally limit it to something like
>      max(1, min(WORKER_CAP, max_connections / 4))
> 
> so that cases like tap tests don't end up allocating vastly more worker slots
> than actual connection slots.

That's a clever idea.  My only concern would be that we are tethering two
parameters that aren't super closely related, but I'm unsure whether it
would cause any problems in practice.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-21T20:44:07Z

On Tue, Jun 18, 2024 at 07:43:36PM -0500, Nathan Bossart wrote:
> On Tue, Jun 18, 2024 at 02:33:31PM -0700, Andres Freund wrote:
>> Another one:
>> 
>> Have a general cap of 64, but additionally limit it to something like
>>      max(1, min(WORKER_CAP, max_connections / 4))
>> 
>> so that cases like tap tests don't end up allocating vastly more worker slots
>> than actual connection slots.
> 
> That's a clever idea.  My only concern would be that we are tethering two
> parameters that aren't super closely related, but I'm unsure whether it
> would cause any problems in practice.

Here is an attempt at doing this.  I've added 0001 [0] and 0002 [1] as
prerequisite patches, which helps simplify 0003 a bit.  It probably doesn't
work correctly for EXEC_BACKEND builds yet.

I'm still not sure about this approach.  At the moment, I'm leaning towards
something more like v2 [2] where the upper limit is a PGC_POSTMASTER GUC
(that we would set very low for TAP tests).

[0] https://commitfest.postgresql.org/48/4998/
[1] https://commitfest.postgresql.org/48/5059/
[2] https://postgr.es/m/20240419154322.GA3988554%40nathanxps13

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-06-22T20:42:47Z

On Fri, Jun 21, 2024 at 03:44:07PM -0500, Nathan Bossart wrote:
> I'm still not sure about this approach.  At the moment, I'm leaning towards
> something more like v2 [2] where the upper limit is a PGC_POSTMASTER GUC
> (that we would set very low for TAP tests).

Like so.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-07-08T19:29:16Z

Here is a rebased patch.

One thing that still bugs me is that there is no feedback sent to the user
when autovacuum_max_workers is set higher than autovacuum_worker_slots.  I
think we should at least emit a WARNING, perhaps from the autovacuum
launcher, i.e., once when the launcher starts and then again as needed via
HandleAutoVacLauncherInterrupts().  Or we could fail to start in
PostmasterMain() and then ignore later misconfigurations via a GUC check
hook.  I'm not too thrilled about adding more GUC check hooks that depend
on the value of other GUCs, but I do like the idea of failing instead of
silently proceeding with a different value than the user configured.  Any
thoughts?

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-07-19T19:24:39Z

On Mon, Jul 08, 2024 at 02:29:16PM -0500, Nathan Bossart wrote:
> One thing that still bugs me is that there is no feedback sent to the user
> when autovacuum_max_workers is set higher than autovacuum_worker_slots.  I
> think we should at least emit a WARNING, perhaps from the autovacuum
> launcher, i.e., once when the launcher starts and then again as needed via
> HandleAutoVacLauncherInterrupts().  Or we could fail to start in
> PostmasterMain() and then ignore later misconfigurations via a GUC check
> hook.  I'm not too thrilled about adding more GUC check hooks that depend
> on the value of other GUCs, but I do like the idea of failing instead of
> silently proceeding with a different value than the user configured.  Any
> thoughts?

From recent discussions, it sounds like there isn't much appetite for GUC
check hooks that depend on the values of other GUCs.  Here is a new version
of the patch that adds the WARNING described above.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-07-27T20:32:09Z
```
rebased

-- 
nathan
```

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-08-21T19:20:08Z

If there are no remaining concerns, I'd like to move forward with
committing v9 in September's commitfest.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-11-19T17:35:46Z
```
rebased

-- 
nathan
```

Re: allow changing autovacuum_max_workers without restarting

Yogesh Sharma <yogesh.sharma@catprosystems.com> — 2024-11-20T14:48:58Z

The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       tested, failed
Spec compliant:           tested, failed
Documentation:            not tested

Hi,
- Tested patch with check-world.
- Verified CheckAutovacuumWorkerGUCs functionality and the correct WARNING was reported.
- For feature specific testing, I created multiple tables and generated bloat. Expected behavior was witnessed.
Lower autovacuum_worker_slots = 16 setting is better suited to start with.

Thanks
Yogesh

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2024-12-20T19:46:20Z

I think I've been saying I would commit this since August, but now I am
planning to do so first thing in the new year.  In v11 of the patch, I
moved the initial startup WARNING to autovac_init() to avoid repeatedly
logging when the launcher restarts (e.g., for emergency vacuums when the
autovacuum parameter is disabled).  Otherwise, I just made a couple of
cosmetic alterations and added a commit message.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-06T15:52:48Z

On Fri, Dec 20, 2024 at 01:46:20PM -0600, Nathan Bossart wrote:
> I think I've been saying I would commit this since August, but now I am
> planning to do so first thing in the new year.  In v11 of the patch, I
> moved the initial startup WARNING to autovac_init() to avoid repeatedly
> logging when the launcher restarts (e.g., for emergency vacuums when the
> autovacuum parameter is disabled).  Otherwise, I just made a couple of
> cosmetic alterations and added a commit message.

This one needed a rebase after commit 2bdf1b2.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-06T21:03:15Z
```
Committed.

-- 
nathan
```

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-06T21:29:37Z

Nathan Bossart <nathandbossart@gmail.com> writes:
> Committed.

Unsurprisingly, this has completely broken buildfarm member sawshark:
you added 13 new semaphores to the system's default requirements,
and we only had headroom for about 4 (cf. 38da05346).

Now maybe we should just abandon the notion that we ought to be
able to start up under OpenBSD/NetBSD's tiny default value of SEMMNS.
If so I'd be inclined to go revert 38da05346, at least in HEAD.
But I kind of wonder whether this feature actually brings value
commensurate with causing installation problems on real-world OSes.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-06T21:50:24Z

On Mon, Jan 06, 2025 at 04:29:37PM -0500, Tom Lane wrote:
> Unsurprisingly, this has completely broken buildfarm member sawshark:
> you added 13 new semaphores to the system's default requirements,
> and we only had headroom for about 4 (cf. 38da05346).

Oh wow, I missed that the defaults were so low on some systems.

> Now maybe we should just abandon the notion that we ought to be
> able to start up under OpenBSD/NetBSD's tiny default value of SEMMNS.
> If so I'd be inclined to go revert 38da05346, at least in HEAD.
> But I kind of wonder whether this feature actually brings value
> commensurate with causing installation problems on real-world OSes.

I'm obviously biased, but I think it would be unfortunate to block features
like this one because of low settings that would otherwise be unsuitable
for any reasonable production workload.  If we do want to at least support
check-world on these systems, another option could be to simply lower the
default of autovacuum_worker_slots to 7 (or maybe lower).  Of course, that
only helps until the next time more semaphores are required, but that's not
a new problem.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-06T22:15:25Z

On Mon, Jan 06, 2025 at 03:50:24PM -0600, Nathan Bossart wrote:
> I'm obviously biased, but I think it would be unfortunate to block features
> like this one because of low settings that would otherwise be unsuitable
> for any reasonable production workload.  If we do want to at least support
> check-world on these systems, another option could be to simply lower the
> default of autovacuum_worker_slots to 7 (or maybe lower).  Of course, that
> only helps until the next time more semaphores are required, but that's not
> a new problem.

I've attached a patch to lower the default to 5.  That at least gives a
little bit of wiggle room for autovacuum_max_workers (and for a couple of
new auxiliary processes).  FWIW the reason I originally set the default to
16 was to prevent most users from ever needing to think about adjusting
autovacuum_worker_slots (which requires a restart and is a completely new
parameter that most will be unfamiliar with).

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Andres Freund <andres@anarazel.de> — 2025-01-06T22:17:38Z

Hi, 

On January 6, 2025 5:15:25 PM EST, Nathan Bossart <nathandbossart@gmail.com> wrote:
>On Mon, Jan 06, 2025 at 03:50:24PM -0600, Nathan Bossart wrote:
>> I'm obviously biased, but I think it would be unfortunate to block features
>> like this one because of low settings that would otherwise be unsuitable
>> for any reasonable production workload.  If we do want to at least support
>> check-world on these systems, another option could be to simply lower the
>> default of autovacuum_worker_slots to 7 (or maybe lower).  Of course, that
>> only helps until the next time more semaphores are required, but that's not
>> a new problem.
>
>I've attached a patch to lower the default to 5.  That at least gives a
>little bit of wiggle room for autovacuum_max_workers (and for a couple of
>new auxiliary processes).  FWIW the reason I originally set the default to
>16 was to prevent most users from ever needing to think about adjusting
>autovacuum_worker_slots (which requires a restart and is a completely new
>parameter that most will be unfamiliar with).

How about trying the higher setting first in initdb? On any sane system that won't cost anything because it'll succeed with the higher value.

Greetings, 

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-06T22:36:17Z

Andres Freund <andres@anarazel.de> writes:
> How about trying the higher setting first in initdb? On any sane system that won't cost anything because it'll succeed with the higher value.

That might be a good compromise.  You'd have to think about how
it should interact with initdb's probes for workable values of
max_connections.  My first thought about that is to have initdb
set autovacuum_worker_slots to max_connections / 8 or thereabouts
as it works down the list of max_connections values to try.  Or
you could do something more complicated, but I don't see a reason
to make it too complex.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-06T23:20:22Z

On Mon, Jan 06, 2025 at 05:36:17PM -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> How about trying the higher setting first in initdb? On any sane system
>> that won't cost anything because it'll succeed with the higher value.
> 
> That might be a good compromise.

+1, I like the idea.

> You'd have to think about how
> it should interact with initdb's probes for workable values of
> max_connections.  My first thought about that is to have initdb
> set autovacuum_worker_slots to max_connections / 8 or thereabouts
> as it works down the list of max_connections values to try.  Or
> you could do something more complicated, but I don't see a reason
> to make it too complex.

My first instinct was just to set it to the lowest default we'd consider
during the max_connections tests (which I'm assuming is 3 due to the
current default for autovacuum_max_workers).  That way, the max_connections
default won't change from version to version on affected systems, but you
might get some extra autovacuum slots.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-06T23:36:43Z

Nathan Bossart <nathandbossart@gmail.com> writes:
> On Mon, Jan 06, 2025 at 05:36:17PM -0500, Tom Lane wrote:
>> You'd have to think about how
>> it should interact with initdb's probes for workable values of
>> max_connections.  My first thought about that is to have initdb
>> set autovacuum_worker_slots to max_connections / 8 or thereabouts
>> as it works down the list of max_connections values to try.  Or
>> you could do something more complicated, but I don't see a reason
>> to make it too complex.

> My first instinct was just to set it to the lowest default we'd consider
> during the max_connections tests (which I'm assuming is 3 due to the
> current default for autovacuum_max_workers).  That way, the max_connections
> default won't change from version to version on affected systems, but you
> might get some extra autovacuum slots.

My only objection to this algorithm is it adds cycles to initdb,
in the form of at least one additional "postgres --check" step.
Admittedly that's not hugely expensive, but it'll add up over time
in the buildfarm, and I'm not sure this issue is worth that.
We already changed the max_connections default for affected systems
as a consequence of 38da05346, so I don't think the argument about not
changing it holds much water.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-07T02:44:37Z

On Mon, Jan 06, 2025 at 06:36:43PM -0500, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> My first instinct was just to set it to the lowest default we'd consider
>> during the max_connections tests (which I'm assuming is 3 due to the
>> current default for autovacuum_max_workers).  That way, the max_connections
>> default won't change from version to version on affected systems, but you
>> might get some extra autovacuum slots.
> 
> My only objection to this algorithm is it adds cycles to initdb,
> in the form of at least one additional "postgres --check" step.
> Admittedly that's not hugely expensive, but it'll add up over time
> in the buildfarm, and I'm not sure this issue is worth that.
> We already changed the max_connections default for affected systems
> as a consequence of 38da05346, so I don't think the argument about not
> changing it holds much water.

I see.  Here's a version that uses your max_connections / 8 idea.  I've
lowered the initial default value of autovacuum_worker_slots to 12 to keep
the code as simple as possible.  I considered trying 16 in the first
iteration or constructing a complicated formula like

	autovacuum_worker_slots = (max_connections * 13) / 75 - 1

but this stuff is pretty fragile already, so I felt that simplicity was
desirable in this case.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-07T03:29:07Z

Nathan Bossart <nathandbossart@gmail.com> writes:
> On Mon, Jan 06, 2025 at 06:36:43PM -0500, Tom Lane wrote:
>> We already changed the max_connections default for affected systems
>> as a consequence of 38da05346, so I don't think the argument about not
>> changing it holds much water.

> I see.  Here's a version that uses your max_connections / 8 idea.  I've
> lowered the initial default value of autovacuum_worker_slots to 12 to keep
> the code as simple as possible.  I considered trying 16 in the first
> iteration or constructing a complicated formula like
> 	autovacuum_worker_slots = (max_connections * 13) / 75 - 1
> but this stuff is pretty fragile already, so I felt that simplicity was
> desirable in this case.

+1 for simplicity ... but on reflection, what do you think about
using max_connections / 6?  That would keep autovacuum_worker_slots
at 100 / 6 = 16 for the vast majority of systems.  For the worst case
*BSD machines, we'd select 25 / 6 = 4 which results in consuming one
more semaphore than where we were yesterday.  I'm willing to accept
that outcome though, since we still have 3 or so to spare.

Other than the specific magic number, your patch LGTM.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-07T17:23:47Z

On Mon, Jan 06, 2025 at 10:29:07PM -0500, Tom Lane wrote:
> +1 for simplicity ... but on reflection, what do you think about
> using max_connections / 6?  That would keep autovacuum_worker_slots
> at 100 / 6 = 16 for the vast majority of systems.  For the worst case
> *BSD machines, we'd select 25 / 6 = 4 which results in consuming one
> more semaphore than where we were yesterday.  I'm willing to accept
> that outcome though, since we still have 3 or so to spare.

WFM.  I'm kicking myself for not having thought of that...

> Other than the specific magic number, your patch LGTM.

Here's a new version of the patch with some small cosmetic changes
(including more commentary about the formula) and the constant changed to
6.  I'll go commit this shortly.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-07T19:22:42Z

Nathan Bossart <nathandbossart@gmail.com> writes:
> Here's a new version of the patch with some small cosmetic changes
> (including more commentary about the formula) and the constant changed to
> 6.  I'll go commit this shortly.

This one WFM.  Thanks!

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-01-07T20:45:20Z

On Tue, Jan 07, 2025 at 02:22:42PM -0500, Tom Lane wrote:
> This one WFM.  Thanks!

Committed, thanks for the report/review.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Peter Eisentraut <peter@eisentraut.org> — 2025-04-28T11:55:48Z

On 07.01.25 18:23, Nathan Bossart wrote:
> +	/*
> +	 * We chose the default for autovacuum_worker_slots during the
> +	 * max_connections tests above, but we print a progress message anyway.
> +	 */
> +	printf(_("selecting default \"autovacuum_worker_slots\" ... %d\n"),
> +		   n_av_slots);
> +

This initdb output seems, well, kinda fake, which it is by its own
admission.  Could we do this less fake maybe like this:

     selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16

with the actual wait at the "..."?

(It doesn't seem impossible that someone will want to add more default
selecting for various worker or process slots, and this would allow adding
these easily, versus adding more "fake" output lines.)

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-28T13:14:54Z

Peter Eisentraut <peter@eisentraut.org> writes:
> This initdb output seems, well, kinda fake, which it is by its own
> admission.

Agreed.

> Could we do this less fake maybe like this:
>      selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16
> with the actual wait at the "..."?

Perhaps that would be all right ...

> (It doesn't seem impossible that someone will want to add more default
> selecting for various worker or process slots, and this would allow adding
> these easily, versus adding more "fake" output lines.)

... but I can't see this approach scaling to three or four or five
outputs.  The line would get unreasonably long.

My own proposal given the way it works now is to just print
max_connections and not mention autovacuum_worker_slots at all.
Our choice for max_connections is worth reporting, but I don't
feel that everything derived from it needs to be reported.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-04-28T14:41:26Z

On Mon, Apr 28, 2025 at 09:14:54AM -0400, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> This initdb output seems, well, kinda fake, which it is by its own
>> admission.
> 
> Agreed.
> 
>> Could we do this less fake maybe like this:
>>      selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16
>> with the actual wait at the "..."?
> 
> Perhaps that would be all right ...
> 
>> (It doesn't seem impossible that someone will want to add more default
>> selecting for various worker or process slots, and this would allow adding
>> these easily, versus adding more "fake" output lines.)
> 
> ... but I can't see this approach scaling to three or four or five
> outputs.  The line would get unreasonably long.
> 
> My own proposal given the way it works now is to just print
> max_connections and not mention autovacuum_worker_slots at all.
> Our choice for max_connections is worth reporting, but I don't
> feel that everything derived from it needs to be reported.

I'm fine with either of these ideas.  If I had to choose one, I'd just
remove the autovacuum_worker_slots report for the reasons Tom noted.

However, weren't we considering reverting some of this stuff [0]?  I see
that sawshark is now choosing max_connections = 40 and
autovacuum_worker_slots = 6, and since there are no other apparent related
buildfarm failures, I'm assuming that nobody else is testing the 60
semaphores case anymore.

[0] https://postgr.es/m/618497.1742347456%40sss.pgh.pa.us

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Peter Eisentraut <peter@eisentraut.org> — 2025-04-29T06:36:48Z

On 28.04.25 16:41, Nathan Bossart wrote:
> On Mon, Apr 28, 2025 at 09:14:54AM -0400, Tom Lane wrote:
>> Peter Eisentraut <peter@eisentraut.org> writes:
>>> This initdb output seems, well, kinda fake, which it is by its own
>>> admission.
>>
>> Agreed.
>>
>>> Could we do this less fake maybe like this:
>>>       selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16
>>> with the actual wait at the "..."?
>>
>> Perhaps that would be all right ...
>>
>>> (It doesn't seem impossible that someone will want to add more default
>>> selecting for various worker or process slots, and this would allow adding
>>> these easily, versus adding more "fake" output lines.)
>>
>> ... but I can't see this approach scaling to three or four or five
>> outputs.  The line would get unreasonably long.
>>
>> My own proposal given the way it works now is to just print
>> max_connections and not mention autovacuum_worker_slots at all.
>> Our choice for max_connections is worth reporting, but I don't
>> feel that everything derived from it needs to be reported.
> 
> I'm fine with either of these ideas.  If I had to choose one, I'd just
> remove the autovacuum_worker_slots report for the reasons Tom noted.

Yes, removing the report is also fine by me.

> However, weren't we considering reverting some of this stuff [0]?  I see
> that sawshark is now choosing max_connections = 40 and
> autovacuum_worker_slots = 6, and since there are no other apparent related
> buildfarm failures, I'm assuming that nobody else is testing the 60
> semaphores case anymore.
> 
> [0] https://postgr.es/m/618497.1742347456%40sss.pgh.pa.us

(I don't have any thoughts on this.)

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-04-29T16:43:10Z

On Tue, Apr 29, 2025 at 08:36:48AM +0200, Peter Eisentraut wrote:
> On 28.04.25 16:41, Nathan Bossart wrote:
>> On Mon, Apr 28, 2025 at 09:14:54AM -0400, Tom Lane wrote:
>> > My own proposal given the way it works now is to just print
>> > max_connections and not mention autovacuum_worker_slots at all.
>> > Our choice for max_connections is worth reporting, but I don't
>> > feel that everything derived from it needs to be reported.
>> 
>> I'm fine with either of these ideas.  If I had to choose one, I'd just
>> remove the autovacuum_worker_slots report for the reasons Tom noted.
> 
> Yes, removing the report is also fine by me.

Committed.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-29T17:19:18Z

Peter Eisentraut <peter@eisentraut.org> writes:
> On 28.04.25 16:41, Nathan Bossart wrote:
>> However, weren't we considering reverting some of this stuff [0]?  I see
>> that sawshark is now choosing max_connections = 40 and
>> autovacuum_worker_slots = 6, and since there are no other apparent related
>> buildfarm failures, I'm assuming that nobody else is testing the 60
>> semaphores case anymore.
>> 
>> [0] https://postgr.es/m/618497.1742347456%40sss.pgh.pa.us

> (I don't have any thoughts on this.)

Andres seemed lukewarm about reverting 38da05346 or 6d0154196, so
I left it be for the moment.  But I still feel the argument is good
that "these will do little except confuse future hackers".  Barring
objection, I'll go revert them.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-04-29T17:24:06Z

On Tue, Apr 29, 2025 at 01:19:18PM -0400, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> On 28.04.25 16:41, Nathan Bossart wrote:
>>> However, weren't we considering reverting some of this stuff [0]?  I see
>>> that sawshark is now choosing max_connections = 40 and
>>> autovacuum_worker_slots = 6, and since there are no other apparent related
>>> buildfarm failures, I'm assuming that nobody else is testing the 60
>>> semaphores case anymore.
>>> 
>>> [0] https://postgr.es/m/618497.1742347456%40sss.pgh.pa.us
> 
>> (I don't have any thoughts on this.)
> 
> Andres seemed lukewarm about reverting 38da05346 or 6d0154196, so
> I left it be for the moment.  But I still feel the argument is good
> that "these will do little except confuse future hackers".  Barring
> objection, I'll go revert them.

+1, I almost threatened the same but wasn't totally positive where the
discussion stood.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-29T17:31:55Z

I wrote:
> Andres seemed lukewarm about reverting 38da05346 or 6d0154196, so
> I left it be for the moment.  But I still feel the argument is good
> that "these will do little except confuse future hackers".  Barring
> objection, I'll go revert them.

Actually ... on looking again at 6d0154196 ("Lower default value of
autovacuum_worker_slots in initdb as needed"), it doesn't look that
silly.  If we're unable to allocate max_connections = 100, turning
it down while still insisting on 16 AV worker slots doesn't seem
terribly sane.  Maybe we'd choose a formula other than
"(max_connections / 6)" if we were doing it afresh, but not scaling
autovacuum_worker_slots at all doesn't seem like the best answer.

So now I'm inclined to leave that one alone.  I'd still revert
38da05346, which means the comment added by 6d0154196 needs some minor
adjustments.  But I think we can stick with the "(max_connections /
6)" formula --- it will produce 3 with trial_conns = 20, but that's
enough.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-04-29T17:42:19Z

On Tue, Apr 29, 2025 at 01:31:55PM -0400, Tom Lane wrote:
> I wrote:
>> Andres seemed lukewarm about reverting 38da05346 or 6d0154196, so
>> I left it be for the moment.  But I still feel the argument is good
>> that "these will do little except confuse future hackers".  Barring
>> objection, I'll go revert them.
> 
> Actually ... on looking again at 6d0154196 ("Lower default value of
> autovacuum_worker_slots in initdb as needed"), it doesn't look that
> silly.  If we're unable to allocate max_connections = 100, turning
> it down while still insisting on 16 AV worker slots doesn't seem
> terribly sane.  Maybe we'd choose a formula other than
> "(max_connections / 6)" if we were doing it afresh, but not scaling
> autovacuum_worker_slots at all doesn't seem like the best answer.

Fair point.

> So now I'm inclined to leave that one alone.  I'd still revert
> 38da05346, which means the comment added by 6d0154196 needs some minor
> adjustments.  But I think we can stick with the "(max_connections /
> 6)" formula --- it will produce 3 with trial_conns = 20, but that's
> enough.

Yup, as long as the lowest possible default is >= the default for
autovacuum_max_workers (3), we're good.

-- 
nathan

Re: allow changing autovacuum_max_workers without restarting

Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-29T21:31:01Z

Nathan Bossart <nathandbossart@gmail.com> writes:
> On Tue, Apr 29, 2025 at 01:31:55PM -0400, Tom Lane wrote:
>> So now I'm inclined to leave that one alone.  I'd still revert
>> 38da05346, which means the comment added by 6d0154196 needs some minor
>> adjustments.  But I think we can stick with the "(max_connections /
>> 6)" formula --- it will produce 3 with trial_conns = 20, but that's
>> enough.

> Yup, as long as the lowest possible default is >= the default for
> autovacuum_max_workers (3), we're good.

Pushed.  I realized that the text about SEMMNI/SEMMNS in runtime.sgml
needed some work too, since it still said that increasing them was
optional on NetBSD/OpenBSD.

			regards, tom lane

Re: allow changing autovacuum_max_workers without restarting

Nathan Bossart <nathandbossart@gmail.com> — 2025-04-29T21:36:20Z

On Tue, Apr 29, 2025 at 05:31:01PM -0400, Tom Lane wrote:
> Pushed.  I realized that the text about SEMMNI/SEMMNS in runtime.sgml
> needed some work too, since it still said that increasing them was
> optional on NetBSD/OpenBSD.

Thanks.  Your updates look good to me.

-- 
nathan