Re: Fix stale comment in parallel_vacuum_main().

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix stale comment in parallel_vacuum_main().
Date: 2026-07-29 06:00:19
Message-ID: C696D03D-5D85-4B8B-B171-342821C98A6A@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jul 29, 2026, at 02:24, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Tue, Jul 28, 2026 at 11:00 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>
>> On Mon, Jul 27, 2026 at 6:00 PM Bharath Rupireddy
>> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>>
>>> Hi,
>>>
>>> While working on the parallel heap vacuum feature I noticed a stale
>>> comment in parallel_vacuum_main(). It says a parallel vacuum worker
>>> must have only PROC_IN_VACUUM set because parallel index vacuum is not
>>> supported for autovacuum. That is no longer true after commit
>>> 1ff3180ca01, which enabled autovacuum to use parallel vacuum workers.
>>
>> Good catch!
>>
>>> IMHO, the assertion itself is still worth keeping. It ensures the
>>> parallel worker carries PROC_IN_VACUUM, so that other backends ignore
>>> its xmin when computing the removable-tuple horizon, which is what we
>>> want for a worker that is part of a vacuum. The leader, whether it is
>>> a backend running a VACUUM command or an autovacuum worker, sets
>>> PROC_IN_VACUUM when it starts vacuuming the table, and the worker
>>> inherits it when it imports the leader's snapshot (see
>>> ProcArrayInstallRestoredXmin). PROC_IS_AUTOVACUUM never reaches the
>>> worker, because parallel workers are launched as regular background
>>> workers rather than autovacuum workers, so the flag is never set on
>>> them, and importing the leader's snapshot doesn't carry it over
>>> either. So the worker ends up with exactly PROC_IN_VACUUM even under
>>> parallel autovacuum, and the assertion holds.
>>>
>>> The attached patch rewords the comment to explain this. Since PG19 is
>>> still in beta, I would like to backpatch this to 19 where 1ff3180ca01
>>> was introduced.
>>>
>>> Thoughts?
>>
>> I agree with your analysis. And the proposed comment looks clear to
>> me. Will push the patch barring any objections.
>>
>
> After reviewing the new comment closely, I think it's better to
> mention the PROC_VACUUM_FOR_WRAPAROUND flag too as this is also used
> for autovacuums. I've attached the updated patch, please review it.
>
> Regards,
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
> <v2-0001-Fix-stale-comment-in-parallel_vacuum_main.patch>

I checked the code. An autovacuum leader already has PROC_IS_AUTOVACUUM. Then vacuum_rel() adds PROC_IN_VACUUM and may also add PROC_VACUUM_FOR_WRAPAROUND when params.is_wraparound is true. The parallel worker starts without any of these flags, and snapshot restoration copies only the PROC_XMIN_FLAGS bits. Since the vacuum leader has PROC_IN_VACUUM but not PROC_IN_SAFE_IC, the worker ends up with only PROC_IN_VACUUM.

So, v2 looks good to me.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2026-07-29 06:21:18 Re: remove_useless_joins vs. bug #19560
Previous Message Ayush Tiwari 2026-07-29 05:41:24 Re: Prove a NOT IN's left-hand expressions non-nullable from quals