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 <huku...@gmail.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@postgres=# set role regress_vacuum_conflict; > SET > Time: 0.369 ms > postgres@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@postgres=> > > Looks like it's a subject for a patch. > > On Mon, Jan 16, 2023 at 11:18 AM Alexander Pyhalov < > a.pyha...@postgrespro.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/
0001_expand_vac_rel_part_chk_v1.patch
Description: Binary data
