Re: POC: Parallel processing of indexes in autovacuum

Hi,

On Wed, May 21, 2025 at 5:30 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I have some comments on v2-0001 patch

Thank you for reviewing this patch!

> + {
> + {"autovacuum_reserved_workers_num", PGC_USERSET,
> RESOURCES_WORKER_PROCESSES,
> + gettext_noop("Number of worker processes, reserved for
> participation in parallel index processing during autovacuum."),
> + gettext_noop("This parameter is depending on
> \"max_worker_processes\" (not on \"autovacuum_max_workers\"). "
> + "*Only* autovacuum workers can use these
> additional processes. "
> + "Also, these processes are taken into account
> in \"max_parallel_workers\"."),
> + },
> + &av_reserved_workers_num,
> + 0, 0, MAX_BACKENDS,
> + check_autovacuum_reserved_workers_num, NULL, NULL
> + },
>
> I find that the name "autovacuum_reserved_workers_num" is generic. It
> would be better to have a more specific name for parallel vacuum such
> as autovacuum_max_parallel_workers. This parameter is related to
> neither autovacuum_worker_slots nor autovacuum_max_workers, which
> seems fine to me. Also, max_parallel_maintenance_workers doesn't
> affect this parameter.
> .......
> I've also considered some alternative names. If we were to use
> parallel_maintenance_workers, it sounds like it controls the parallel
> degree for all operations using max_parallel_maintenance_workers,
> including CREATE INDEX. Similarly, vacuum_parallel_workers could be
> interpreted as affecting both autovacuum and manual VACUUM commands,
> suggesting that when users run "VACUUM (PARALLEL) t", the system would
> use their specified value for the parallel degree. I prefer
> autovacuum_parallel_workers or vacuum_parallel_workers.
>

This was my headache when I created names for variables. Autovacuum
initially implies parallelism, because we have several parallel a/v
workers. So I think that parameter like
`autovacuum_max_parallel_workers` will confuse somebody.
If we want to have a more specific name, I would prefer
`max_parallel_index_autovacuum_workers`.

> Which number does this parameter mean to specify: the maximum number
> of parallel vacuum workers that can be used during autovacuum or the
> maximum number of parallel vacuum workers that each autovacuum can
> use?

First variant. I will concrete this in the variable's description.

> + {
> + {
> + "parallel_idx_autovac_enabled",
> + "Allows autovacuum to process indexes of this table in
> parallel mode",
> + RELOPT_KIND_HEAP,
> + ShareUpdateExclusiveLock
> + },
> + false
> + },
>
> The proposed reloption name doesn't align with our naming conventions.
> Looking at our existing reloptions, we typically write out full words
> rather than using abbreviations like 'autovac' or 'idx'.
>
> The new reloption name seems not to follow the conventional naming
> style for existing reloption. For instance, we don't use abbreviations
> such as 'autovac' and 'idx'.

OK, I'll fix it.

> + /*
> + * If we are running autovacuum - decide whether we need to process indexes
> + * of table with given oid in parallel.
> + */
> + if (AmAutoVacuumWorkerProcess() &&
> + params->index_cleanup != VACOPTVALUE_DISABLED &&
> + RelationAllowsParallelIdxAutovac(rel))
>
> I think that this should be done in autovacuum code.

We need params->index cleanup variable to decide whether we need to
use parallel index a/v. In autovacuum.c we have this code :
***
/*
* index_cleanup and truncate are unspecified at first in autovacuum.
* They will be filled in with usable values using their reloptions
* (or reloption defaults) later.
*/
tab->at_params.index_cleanup = VACOPTVALUE_UNSPECIFIED;
tab->at_params.truncate = VACOPTVALUE_UNSPECIFIED;
***
This variable is filled in inside the `vacuum_rel` function, so I
think we should keep the above logic in vacuum.c.

> +#define AV_PARALLEL_DEADTUP_THRESHOLD 1024
>
> These fixed values really useful in common cases? I think we already
> have an optimization where we skip vacuum indexes if the table has
> fewer dead tuples (see BYPASS_THRESHOLD_PAGES).

When we allocate dead items (and optionally init parallel autocuum) we
don't have sane value for `vacrel->lpdead_item_pages` (which should be
compared with BYPASS_THRESHOLD_PAGES).
The only criterion that we can focus on is the number of dead tuples
indicated in the PgStat_StatTabEntry.

----

> I guess we can implement this parameter as an integer parameter so
> that the user can specify the number of parallel vacuum workers for
> the table. For example, we can have a reloption
> autovacuum_parallel_workers. Setting 0 (by default) means to disable
> parallel vacuum during autovacuum, and setting special value -1 means
> to let PostgreSQL calculate the parallel degree for the table (same as
> the default VACUUM command behavior).
> ...........
> The patch includes the changes to bgworker.c so that we can reserve
> some slots for autovacuums. I guess that this change is not
> necessarily necessary because if the user sets the related GUC
> parameters correctly the autovacuum workers can use parallel vacuum as
> expected. Even if we need this change, I would suggest implementing
> it as a separate patch.
> ..........
> +#define AV_PARALLEL_DEADTUP_THRESHOLD 1024
> +#define NUM_INDEXES_PER_PARALLEL_WORKER 30
>
> These fixed values really useful in common cases? Given that we rely on
> users' heuristics which table needs to use parallel vacuum during
> autovacuum, I think we don't need to apply these conditions.
> ..........

I grouped these comments together, because they all relate to a single
question : how much freedom will we give to the user?
Your opinion (as far as I understand) is that we allow users to
specify any number of parallel workers for tables, and it is the
user's responsibility to configure appropriate GUC variables, so that
autovacuum can always process indexes in parallel.
And we don't need to think about thresholds. Even if the table has a
small number of indexes and dead rows - if the user specified table
option, we must do a parallel index a/v with requested number of
parallel workers.
Please correct me if I messed something up.

I think that this logic is well suited for the `VACUUM (PARALLEL)` sql
command, which is manually called by the user.

But autovacuum (as I think) should work as stable as possible and
`unnoticed` by other processes. Thus, we must :
1) Compute resources (such as the number of parallel workers for a
single table's indexes vacuuming) as efficiently as possible.
2) Provide a guarantee that as many tables as possible (among
requested) will be processed in parallel.

(1) can be achieved by calculating the parameters on the fly.
NUM_INDEXES_PER_PARALLEL_WORKER is a rough mock. I can provide more
accurate value in the near future.
(2) can be achieved by workers reserving - we know that N workers
(from bgworkers pool) are *always* at our disposal. And when we use
such workers we are not dependent on other operations in the cluster
and we don't interfere with other operations by taking resources away
from them.

If we give the user too much freedom in parallel index a/v tuning, all
these requirements may be violated.

This is only my opinion, and I can agree with yours. Maybe we need
another person to judge us?
Please see v3 patches that contain changes related to GUC parameter
and table option (no changes in global logic by now).

--
Best regards,
Daniil Davydov