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