Re: Inconsistency in vacuum behavior

Hi!

Here's the patch that fixes this case, please check it out.
The patch adds vacuum_is_permitted_for_relation() check before adding
partition relation to the vacuum list, and if permission is denied the
relation
is not added, so it is not passed to vacuum_rel() and there are no try to
acquire the lock.

Cheers!

On Mon, Jan 16, 2023 at 4:48 PM Nikita Malakhov <hukutoc(at)gmail(dot)com> wrote:

> Hi!
>
> I've checked this expand_vacuum_rel() and made a quick fix for this.Here's
> the result of the test:
>
> postgres(at)postgres=# set role regress_vacuum_conflict;
> SET
> Time: 0.369 ms
> postgres(at)postgres=> vacuum vacuum_tab;
> WARNING: permission denied to vacuum "vacuum_tab", skipping it
> WARNING: permission denied to vacuum "vacuum_tab_1", skipping it
> WARNING: permission denied to vacuum "vacuum_tab_2", skipping it
> VACUUM
> Time: 0.936 ms
> postgres(at)postgres=>
>
> Looks like it's a subject for a patch.
>
> On Mon, Jan 16, 2023 at 11:18 AM Alexander Pyhalov <
> a(dot)pyhalov(at)postgrespro(dot)ru> wrote:
>
>> Hi.
>>
>> We've run regress isolation tests on partitioned tables and found
>> interesting VACUUM behavior. I'm not sure, if it's intended.
>>
>> In the following example, partitioned tables and regular tables behave
>> differently:
>>
>> CREATE TABLE vacuum_tab (a int) PARTITION BY HASH (a);
>> CREATE TABLE vacuum_tab_1 PARTITION OF vacuum_tab FOR VALUES WITH
>> (MODULUS 2, REMAINDER 0);
>> CREATE TABLE vacuum_tab_2 PARTITION OF vacuum_tab FOR VALUES WITH
>> (MODULUS 2, REMAINDER 1);
>> CREATE ROLE regress_vacuum_conflict;
>>
>> In the first session:
>>
>> begin;
>> LOCK vacuum_tab IN SHARE UPDATE EXCLUSIVE MODE;
>>
>> In the second:
>> SET ROLE regress_vacuum_conflict;
>> VACUUM vacuum_tab;
>> WARNING: permission denied to vacuum "vacuum_tab", skipping it <----
>> hangs here, trying to lock vacuum_tab_1
>>
>> In non-partitioned case second session exits after emitting warning. In
>> partitioned case, it hangs, trying to get locks.
>> This is due to the fact that in expand_vacuum_rel() we skip parent table
>> if vacuum_is_permitted_for_relation(), but don't perform such check for
>> its child.
>> The check will be performed later in vacuum_rel(), but after
>> vacuum_open_relation(), which leads to hang in the second session.
>>
>> Is it intended? Why don't we perform vacuum_is_permitted_for_relation()
>> check for inheritors in expand_vacuum_rel()?
>>
>> --
>> Best regards,
>> Alexander Pyhalov,
>> Postgres Professional
>>
>>
>>
>
> --
> Regards,
> Nikita Malakhov
> Postgres Professional
> https://postgrespro.ru/
>

--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/