agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedHandle concurrent drop when doing whole database vacuum
44+ messages / 6 participants
[nested] [flat]
* Handle concurrent drop when doing whole database vacuum
@ 2026-06-14 07:12 =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-17 18:40 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 3 replies; 44+ messages in thread
From: cca5507 @ 2026-06-14 07:12 UTC (permalink / raw)
To: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi hackers,
When doing a whole database vacuum, we scan pg_class to construct
a list of vacuumable tables. For each vacuumable table, we call
vacuum_is_permitted_for_relation() to check permissions. If a
concurrent drop happens, the pg_class_aclcheck() might report an
error because of failing to search the syscache:
ERROR: relation with OID ****** does not exist
To fix it, we can use pg_class_aclcheck_ext() to detect the concurrent
drop and report a warning instead.
Note that a concurrent drop after constructing the list of vacuumable
tables is handled by vacuum_open_relation().
Thoughts?
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v1-0001-Handle-concurrent-drop-when-doing-whole-database-.patch (2.5K, ../../tencent_F9D483523BB0D082C2EFDA80142F192DBC07@qq.com/2-v1-0001-Handle-concurrent-drop-when-doing-whole-database-.patch)
download | inline diff:
From 8ba82796a82354a10ab5adedfec0d823d07eefd8 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Sun, 14 Jun 2026 14:37:12 +0800
Subject: [PATCH v1] Handle concurrent drop when doing whole database vacuum.
When doing a whole database vacuum, we scan pg_class to construct
a list of vacuumable tables. For each vacuumable table, we call
vacuum_is_permitted_for_relation() to check permissions. If a
concurrent drop happens, the pg_class_aclcheck() might report an
error because of failing to search the syscache.
To fix it, we use pg_class_aclcheck_ext() to detect the concurrent
drop and report a warning instead.
---
src/backend/commands/vacuum.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..4291cb8410c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -721,6 +721,7 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
uint32 options)
{
char *relname;
+ bool is_missing = false;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
@@ -733,16 +734,21 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
!reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+ pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
return true;
relname = NameStr(reltuple->relname);
if ((options & VACOPT_VACUUM) != 0)
{
- ereport(WARNING,
- (errmsg("permission denied to vacuum \"%s\", skipping it",
- relname)));
+ if (is_missing)
+ ereport(WARNING,
+ (errmsg("skipping vacuum of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to vacuum \"%s\", skipping it",
+ relname)));
/*
* For VACUUM ANALYZE, both logs could show up, but just generate
@@ -753,9 +759,16 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
}
if ((options & VACOPT_ANALYZE) != 0)
- ereport(WARNING,
- (errmsg("permission denied to analyze \"%s\", skipping it",
- relname)));
+ {
+ if (is_missing)
+ ereport(WARNING,
+ (errmsg("skipping analyze of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to analyze \"%s\", skipping it",
+ relname)));
+ }
return false;
}
--
2.54.0
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-15 05:54 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2 siblings, 1 reply; 44+ messages in thread
From: Kyotaro Horiguchi @ 2026-06-15 05:54 UTC (permalink / raw)
To: cca5507@qq.com; +Cc: pgsql-hackers@lists.postgresql.org
Hello.
At Sun, 14 Jun 2026 15:12:43 +0800, "cca5507" <cca5507@qq.com> wrote in
> When doing a whole database vacuum, we scan pg_class to construct
> a list of vacuumable tables. For each vacuumable table, we call
> vacuum_is_permitted_for_relation() to check permissions. If a
> concurrent drop happens, the pg_class_aclcheck() might report an
> error because of failing to search the syscache:
>
> ERROR: relation with OID ****** does not exist
Good catch!
> To fix it, we can use pg_class_aclcheck_ext() to detect the concurrent
> drop and report a warning instead.
Another possible direction might be to take a lock when building the
list, instead of dealing with the race later. For example, the repack
command seems to take a light lock when it finds a candidate relation:
static List *
get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
> /*
> * Try to obtain a light lock on the table, to ensure it doesn't
> * go away while we collect the list. If we cannot, just
> * disregard the table. Be sure to release this if we ultimately
> * decide not to process the table!
> */
> if (!ConditionalLockRelationOid(class->oid, AccessShareLock))
> continue;
Whether a relation that disappears immediately after being added to
the list should be processed or skipped does not seem particularly
important in practice. However, taking a lock at list construction
time may make the subsequent processing simpler. I wonder whether that
would be a reasonable direction for VACUUM as well.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2026-06-15 06:29 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-15 06:29 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi Kyotaro,
> > To fix it, we can use pg_class_aclcheck_ext() to detect the concurrent
> > drop and report a warning instead.
>
> Another possible direction might be to take a lock when building the
> list, instead of dealing with the race later. For example, the repack
> command seems to take a light lock when it finds a candidate relation:
>
> static List *
> get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
> > /*
> > * Try to obtain a light lock on the table, to ensure it doesn't
> > * go away while we collect the list. If we cannot, just
> > * disregard the table. Be sure to release this if we ultimately
> > * decide not to process the table!
> > */
> > if (!ConditionalLockRelationOid(class->oid, AccessShareLock))
> > continue;
>
> Whether a relation that disappears immediately after being added to
> the list should be processed or skipped does not seem particularly
> important in practice. However, taking a lock at list construction
> time may make the subsequent processing simpler. I wonder whether that
> would be a reasonable direction for VACUUM as well.
I don't think it's a good idea because there might be a lot of tables need
to be locked. And a concurrent drop can always happen after the list
construction because we process each table in a separate transaction.
So I think there is no need to prevent concurrent drops at list construction
time. I also prepare to write a patch to remove the lock in get_tables_to_repack().
--
Regards,
ChangAo Chen
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-23 23:13 ` Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Michael Paquier @ 2026-06-23 23:13 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Mon, Jun 15, 2026 at 02:29:57PM +0800, cca5507 wrote:
>> Whether a relation that disappears immediately after being added to
>> the list should be processed or skipped does not seem particularly
>> important in practice. However, taking a lock at list construction
>> time may make the subsequent processing simpler. I wonder whether that
>> would be a reasonable direction for VACUUM as well.
>
> I don't think it's a good idea because there might be a lot of tables need
> to be locked. And a concurrent drop can always happen after the list
> construction because we process each table in a separate transaction.
> So I think there is no need to prevent concurrent drops at list construction
> time. I also prepare to write a patch to remove the lock in get_tables_to_repack().
Yeah, I doubt that forcing a lock an extra lock at an early stage is a
good thing for a manual VACUUM.
Anyway, I am wondering if we should aim for simpler. Do we really
need the extra ACL check when building a list of relations to consider
for a manual VACUUM in the pg_class scan? We are going to re-check
the permissions once we vacuum each relation in its own transaction,
*after* taking a lock on them, making the ACL check safe. That's the
vacuum_open_relation()->vacuum_is_permitted_* flow.
On a database with many relations where there is no MAINTAIN privilege
for the role running the manual VACUUM, it means more transaction
overhead because more transactions would need to be created for each
relation whose ACLs need to be rechecked, because we don't filter the
relations beforehand with the initial pg_class scan, but that would
protect from the concurrent drops entirely by limitting the check to
be in *one* path: the one analyzing or vacuuming a single relation.
I am not saying that any of this should be backpatched and that we
should treat this as a bug, a concurrent DROP reflecting on a
database-wide VACUUM is annoying, but that's not really a critical
thing to deal with. Improving that on HEAD sounds fine to me (not
v19).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../ajsTKldYusgjIzKI@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-23 23:33 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-23 23:33 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Tue, Jun 23, 2026 at 4:14 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> Anyway, I am wondering if we should aim for simpler. Do we really
> need the extra ACL check when building a list of relations to consider
> for a manual VACUUM in the pg_class scan? We are going to re-check
> the permissions once we vacuum each relation in its own transaction,
> *after* taking a lock on them, making the ACL check safe. That's the
> vacuum_open_relation()->vacuum_is_permitted_* flow.
>
> On a database with many relations where there is no MAINTAIN privilege
> for the role running the manual VACUUM, it means more transaction
> overhead because more transactions would need to be created for each
> relation whose ACLs need to be rechecked, because we don't filter the
> relations beforehand with the initial pg_class scan, but that would
> protect from the concurrent drops entirely by limitting the check to
> be in *one* path: the one analyzing or vacuuming a single relation.
Spot on. My thinking was along similar lines. I'm concerned that in
the worst case - where the role running a database-wide vacuum has no
MAINTAIN privilege at all, or has it on only a small subset of tables
- this approach will unnecessarily do a bunch of memory allocations,
start a transaction, get a snapshot, commit the transaction, and
acquire/release locks for each relation. So, IMHO, -1 for this
approach. Instead, I would prefer using the pg_class_aclcheck_ext
check and just emitting a WARNING (like the other skipping messages)
when the relation is concurrently dropped.
That said, I'm open to hearing from others.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-23 23:40 ` Michael Paquier <michael@paquier.xyz>
2026-06-24 03:21 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:54 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 44+ messages in thread
From: Michael Paquier @ 2026-06-23 23:40 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Nathan Bossart <nathandbossart@gmail.com>; Jeff Davis <pgsql@j-davis.com>
On Tue, Jun 23, 2026 at 04:33:11PM -0700, Bharath Rupireddy wrote:
> On Tue, Jun 23, 2026 at 4:14 PM Michael Paquier <michael@paquier.xyz> wrote:
>>
>> Anyway, I am wondering if we should aim for simpler. Do we really
>> need the extra ACL check when building a list of relations to consider
>> for a manual VACUUM in the pg_class scan? We are going to re-check
>> the permissions once we vacuum each relation in its own transaction,
>> *after* taking a lock on them, making the ACL check safe. That's the
>> vacuum_open_relation()->vacuum_is_permitted_* flow.
>>
>> On a database with many relations where there is no MAINTAIN privilege
>> for the role running the manual VACUUM, it means more transaction
>> overhead because more transactions would need to be created for each
>> relation whose ACLs need to be rechecked, because we don't filter the
>> relations beforehand with the initial pg_class scan, but that would
>> protect from the concurrent drops entirely by limitting the check to
>> be in *one* path: the one analyzing or vacuuming a single relation.
>
> Spot on. My thinking was along similar lines. I'm concerned that in
> the worst case - where the role running a database-wide vacuum has no
> MAINTAIN privilege at all, or has it on only a small subset of tables
> - this approach will unnecessarily do a bunch of memory allocations,
> start a transaction, get a snapshot, commit the transaction, and
> acquire/release locks for each relation. So, IMHO, -1 for this
> approach. Instead, I would prefer using the pg_class_aclcheck_ext
> check and just emitting a WARNING (like the other skipping messages)
> when the relation is concurrently dropped.
>
> That said, I'm open to hearing from others.
I tend to think that you are worrying too much. If a user has a small
subset of tables, they could just run a VACUUM with a list of tables
instead. That's a tradeoff of code simplicity vs cost, and I'm
finding the simplicity argument quite tempting here.
I'd sure welcome Nathan and Jeff opinions (added now in CC) regarding
this line of thoughts; they worked on MAINTAIN, even if the early
relation ACL check when building a list of relations for a
database-wide manual VACUUM predates that.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../ajsZZlgYYPa37CIz@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-24 03:21 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-24 03:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-24 03:21 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Nathan Bossart <nathandbossart@gmail.com>; Jeff Davis <pgsql@j-davis.com>
Hi Michael,
> I'd sure welcome Nathan and Jeff opinions (added now in CC) regarding
> this line of thoughts; they worked on MAINTAIN, even if the early
> relation ACL check when building a list of relations for a
> database-wide manual VACUUM predates that.
It seems that the original permission check in get_all_vacuum_rels() is added
by you in commit a556549d7e6dce15fe216bd4130ea64239f4d83f.
--
Regards,
ChangAo Chen
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-24 03:21 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-24 03:40 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 44+ messages in thread
From: Michael Paquier @ 2026-06-24 03:40 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Nathan Bossart <nathandbossart@gmail.com>; Jeff Davis <pgsql@j-davis.com>
On Wed, Jun 24, 2026 at 11:21:46AM +0800, cca5507 wrote:
> It seems that the original permission check in get_all_vacuum_rels() is added
> by you in commit a556549d7e6dce15fe216bd4130ea64239f4d83f.
Aye, I'm aware of that. :D
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../ajtRoKXDIkvEiX_h@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-29 16:54 ` Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 04:47 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 44+ messages in thread
From: Nathan Bossart @ 2026-06-29 16:54 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Jeff Davis <pgsql@j-davis.com>
On Wed, Jun 24, 2026 at 08:40:22AM +0900, Michael Paquier wrote:
> I'd sure welcome Nathan and Jeff opinions (added now in CC) regarding
> this line of thoughts; they worked on MAINTAIN, even if the early
> relation ACL check when building a list of relations for a
> database-wide manual VACUUM predates that.
I'm mostly concerned about reopening the ability for folks to take strong
locks on catalogs without the necessary privileges, even if they are only
briefly held. Commit a556549 fixed a real problem, so I'm nervous about
partially reverting it. IOW my first reaction is that v1 is the right
idea, i.e., we should just skip the relation if it disappears. I'm not
sure we even need to log it.
--
nathan
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 16:54 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
@ 2026-06-30 04:47 ` Michael Paquier <michael@paquier.xyz>
2026-06-30 15:30 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Michael Paquier @ 2026-06-30 04:47 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Jeff Davis <pgsql@j-davis.com>
On Mon, Jun 29, 2026 at 11:54:44AM -0500, Nathan Bossart wrote:
> I'm mostly concerned about reopening the ability for folks to take strong
> locks on catalogs without the necessary privileges, even if they are only
> briefly held. Commit a556549 fixed a real problem, so I'm nervous about
> partially reverting it. IOW my first reaction is that v1 is the right
> idea, i.e., we should just skip the relation if it disappears. I'm not
> sure we even need to log it.
I have finally spent some time with my head down on this problem.
This would be an issue when a role issues a database-wide VACUUM but
lacks grant access to a table it may look at when opening the
relation.
Simple example, two sessions with this setup:
create role popo login;
create table vacuum_tab (a int); -- owner is my superuser
1) session 1: superuser role
BEGIN;
LOCK vacuum_tab;
2) session 2: user popo
-- Allowed to run, and all relations should be skipped.
VACUUM;
And unfortunately, my intuition and memories were wrong. If we simply
remove the early ACL check when the list of relations is built, the
system-wide VACUUM would block when trying to open the relation
vacuum_tab, meaning that lock attempts would stack, and that's what
a556549d7e6d is all about: the VACUUM should run, and skip all
relations.
Keeping the early ACL check and switching to _ext() would keep the
safeguard in place when running a database-wide VACUUM, and address
the concurrent drop issue. The suggestion of not logging that the
relation is gone while checking its ACL while we don't hold a lock on
the relation feels OK here.
Something that still feels off to me is to blindly use _ext() in
vacuum_is_permitted_for_relation(), where we *may* already hold a lock
on the relation whose ACL is checked. In this case missing a relation
is not fine, so this would make the code more brittle in the
single-relation case under autovacuum or a VACUUM with a list of
relations provided by a user.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../akNKbT-481DqyNOP@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 16:54 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 04:47 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-30 15:30 ` Nathan Bossart <nathandbossart@gmail.com>
2026-07-09 18:49 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Nathan Bossart @ 2026-06-30 15:30 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Jeff Davis <pgsql@j-davis.com>
On Tue, Jun 30, 2026 at 01:47:41PM +0900, Michael Paquier wrote:
> Something that still feels off to me is to blindly use _ext() in
> vacuum_is_permitted_for_relation(), where we *may* already hold a lock
> on the relation whose ACL is checked. In this case missing a relation
> is not fine, so this would make the code more brittle in the
> single-relation case under autovacuum or a VACUUM with a list of
> relations provided by a user.
Yeah, so we should only use it for get_all_vacuum_rels(), as in the
attached.
--
nathan
From aad9a02072544a7bc2c3f861987748b815fcddc9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 30 Jun 2026 10:29:16 -0500
Subject: [PATCH v8 1/1] handle concurrent drop in database-wide vacuum
---
src/backend/commands/analyze.c | 3 ++-
src/backend/commands/vacuum.c | 23 ++++++++++++++++++-----
src/include/commands/vacuum.h | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index f66e80b757c..c28b9dae983 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -157,7 +157,8 @@ analyze_rel(Oid relid, RangeVar *relation,
*/
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
onerel->rd_rel,
- params->options & ~VACOPT_VACUUM))
+ params->options & ~VACOPT_VACUUM,
+ false))
{
relation_close(onerel, ShareUpdateExclusiveLock);
return;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..a402d2330f0 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -718,9 +718,10 @@ vacuum(List *relations, const VacuumParams *params, BufferAccessStrategy bstrate
*/
bool
vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
- uint32 options)
+ uint32 options, bool missing_ok)
{
char *relname;
+ bool is_missing = false;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
@@ -733,9 +734,20 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
!reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+ pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN,
+ missing_ok ? &is_missing : NULL) == ACLCHECK_OK)
return true;
+ /*
+ * If the relation was concurrently dropped, nothing to do. Note that
+ * this is only reachable when the caller specified missing_ok.
+ */
+ if (is_missing)
+ {
+ Assert(missing_ok);
+ return false;
+ }
+
relname = NameStr(reltuple->relname);
if ((options & VACOPT_VACUUM) != 0)
@@ -956,7 +968,7 @@ expand_vacuum_rel(VacuumRelation *vrel, MemoryContext vac_context,
* Make a returnable VacuumRelation for this rel if the user has the
* required privileges.
*/
- if (vacuum_is_permitted_for_relation(relid, classForm, options))
+ if (vacuum_is_permitted_for_relation(relid, classForm, options, false))
{
oldcontext = MemoryContextSwitchTo(vac_context);
vacrels = lappend(vacrels, makeVacuumRelation(vrel->relation,
@@ -1069,7 +1081,7 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
continue;
/* check permissions of relation */
- if (!vacuum_is_permitted_for_relation(relid, classForm, options))
+ if (!vacuum_is_permitted_for_relation(relid, classForm, options, true))
continue;
/*
@@ -2115,7 +2127,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
*/
if (!vacuum_is_permitted_for_relation(priv_relid,
rel->rd_rel,
- params.options & ~VACOPT_ANALYZE))
+ params.options & ~VACOPT_ANALYZE,
+ false))
{
relation_close(rel, lmode);
PopActiveSnapshot();
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 956d9cea36d..e62f23748dc 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -389,7 +389,7 @@ extern bool vacuum_xid_failsafe_check(const struct VacuumCutoffs *cutoffs);
extern void vac_update_datfrozenxid(void);
extern void vacuum_delay_point(bool is_analyze);
extern bool vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
- uint32 options);
+ uint32 options, bool missing_ok);
extern Relation vacuum_open_relation(Oid relid, RangeVar *relation,
uint32 options, bool verbose,
LOCKMODE lmode);
--
2.50.1 (Apple Git-155)
Attachments:
[text/plain] v8-0001-handle-concurrent-drop-in-database-wide-vacuum.patch (3.9K, ../../akPhEffRipH4isWF@nathan/2-v8-0001-handle-concurrent-drop-in-database-wide-vacuum.patch)
download | inline diff:
From aad9a02072544a7bc2c3f861987748b815fcddc9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 30 Jun 2026 10:29:16 -0500
Subject: [PATCH v8 1/1] handle concurrent drop in database-wide vacuum
---
src/backend/commands/analyze.c | 3 ++-
src/backend/commands/vacuum.c | 23 ++++++++++++++++++-----
src/include/commands/vacuum.h | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index f66e80b757c..c28b9dae983 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -157,7 +157,8 @@ analyze_rel(Oid relid, RangeVar *relation,
*/
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
onerel->rd_rel,
- params->options & ~VACOPT_VACUUM))
+ params->options & ~VACOPT_VACUUM,
+ false))
{
relation_close(onerel, ShareUpdateExclusiveLock);
return;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..a402d2330f0 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -718,9 +718,10 @@ vacuum(List *relations, const VacuumParams *params, BufferAccessStrategy bstrate
*/
bool
vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
- uint32 options)
+ uint32 options, bool missing_ok)
{
char *relname;
+ bool is_missing = false;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
@@ -733,9 +734,20 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
!reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+ pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN,
+ missing_ok ? &is_missing : NULL) == ACLCHECK_OK)
return true;
+ /*
+ * If the relation was concurrently dropped, nothing to do. Note that
+ * this is only reachable when the caller specified missing_ok.
+ */
+ if (is_missing)
+ {
+ Assert(missing_ok);
+ return false;
+ }
+
relname = NameStr(reltuple->relname);
if ((options & VACOPT_VACUUM) != 0)
@@ -956,7 +968,7 @@ expand_vacuum_rel(VacuumRelation *vrel, MemoryContext vac_context,
* Make a returnable VacuumRelation for this rel if the user has the
* required privileges.
*/
- if (vacuum_is_permitted_for_relation(relid, classForm, options))
+ if (vacuum_is_permitted_for_relation(relid, classForm, options, false))
{
oldcontext = MemoryContextSwitchTo(vac_context);
vacrels = lappend(vacrels, makeVacuumRelation(vrel->relation,
@@ -1069,7 +1081,7 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
continue;
/* check permissions of relation */
- if (!vacuum_is_permitted_for_relation(relid, classForm, options))
+ if (!vacuum_is_permitted_for_relation(relid, classForm, options, true))
continue;
/*
@@ -2115,7 +2127,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
*/
if (!vacuum_is_permitted_for_relation(priv_relid,
rel->rd_rel,
- params.options & ~VACOPT_ANALYZE))
+ params.options & ~VACOPT_ANALYZE,
+ false))
{
relation_close(rel, lmode);
PopActiveSnapshot();
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 956d9cea36d..e62f23748dc 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -389,7 +389,7 @@ extern bool vacuum_xid_failsafe_check(const struct VacuumCutoffs *cutoffs);
extern void vac_update_datfrozenxid(void);
extern void vacuum_delay_point(bool is_analyze);
extern bool vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
- uint32 options);
+ uint32 options, bool missing_ok);
extern Relation vacuum_open_relation(Oid relid, RangeVar *relation,
uint32 options, bool verbose,
LOCKMODE lmode);
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 16:54 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 04:47 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-30 15:30 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
@ 2026-07-09 18:49 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-07-09 18:56 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-07-09 18:49 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Jeff Davis <pgsql@j-davis.com>
Hi,
On Tue, Jun 30, 2026 at 8:30 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:
>
> On Tue, Jun 30, 2026 at 01:47:41PM +0900, Michael Paquier wrote:
> > Something that still feels off to me is to blindly use _ext() in
> > vacuum_is_permitted_for_relation(), where we *may* already hold a lock
> > on the relation whose ACL is checked. In this case missing a relation
> > is not fine, so this would make the code more brittle in the
> > single-relation case under autovacuum or a VACUUM with a list of
> > relations provided by a user.
>
> Yeah, so we should only use it for get_all_vacuum_rels(), as in the
> attached.
Thanks for pointing at commit a556549d7e6d. I spent more time on this, and
I wasn't fully aware of what that commit was preventing. The worry I raised
earlier turns out to be related:
https://www.postgresql.org/message-id/CALj2ACV8NWt0AtBd35km0YTCu7%2BforTjpDm09V3HWJRfGAMhoA%40mail.g...
.
My general thinking is this: keep unprivileged users from doing more work
when possible, especially riskier stuff like locking relations, and perhaps
a bunch of comparatively less risky stuff too like memory allocations,
acquiring and releasing proc-array lock, starting a transaction, getting a
snapshot, committing the transaction, etc. The problem of unnecessary
locking of relations by unprivileged users cannot be avoided for vacuum
with a table list because it needs to find the OIDs. For autovacuum this
isn't a concern, since it runs as a bootstrap superuser and passes OIDs
directly, so it clears the privilege check.
Removing ACL checks in get_all_vacuum_rels like in v7 effectively brings
back the problem that a556549d7e6d fixed. For example, v7 allows something
like this to happen:
-- create an unprivileged user with no MAINTAIN role
create role alice login;
-- session 1
begin;
select * from pg_authid;
-- leave the txn open, holds AccessShareLock on pg_authid until commit
-- session 2
-- unprivileged user attempts a database-wide vacuum:
set role alice;
vacuum full;
-- vacuum_rel requests AccessExclusiveLock on pg_authid and gets queued,
-- which conflicts with session 1's AccessShareLock, so alice's request
-- parks in the queue, waiting.
-- This is the strong lock request an unprivileged user should never have
-- been able to place at first.
-- session 3
-- all new connections would block
./psql -U alice -d postgres
So I agree with using the _ext version for the ACL check when building the
relations list for database-wide vacuum. It addresses the concurrent table
drops issue. The v8 patch looks good to me.
Also, I don't have a strong opinion on adding the Assert(missing_ok ||
CheckRelationOidLockedByMe(relid, AccessShareLock, true)); because the
is_missing flag in v8 already conveys whether the caller holds the relation
lock or not: missing_ok = true means no lock held, missing_ok = false means
the caller holds it.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 16:54 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 04:47 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-30 15:30 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
2026-07-09 18:49 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-07-09 18:56 ` Nathan Bossart <nathandbossart@gmail.com>
2026-08-03 21:10 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Nathan Bossart @ 2026-07-09 18:56 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Jeff Davis <pgsql@j-davis.com>
On Thu, Jul 09, 2026 at 11:49:24AM -0700, Bharath Rupireddy wrote:
> So I agree with using the _ext version for the ACL check when building the
> relations list for database-wide vacuum. It addresses the concurrent table
> drops issue. The v8 patch looks good to me.
>
> Also, I don't have a strong opinion on adding the Assert(missing_ok ||
> CheckRelationOidLockedByMe(relid, AccessShareLock, true)); because the
> is_missing flag in v8 already conveys whether the caller holds the relation
> lock or not: missing_ok = true means no lock held, missing_ok = false means
> the caller holds it.
Cool, I'll address everyone's feedback and get this committed in the next
few days.
--
nathan
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 16:54 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 04:47 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-30 15:30 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
2026-07-09 18:49 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-07-09 18:56 ` Re: Handle concurrent drop when doing whole database vacuum Nathan Bossart <nathandbossart@gmail.com>
@ 2026-08-03 21:10 ` Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 44+ messages in thread
From: Nathan Bossart @ 2026-08-03 21:10 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Jeff Davis <pgsql@j-davis.com>
On Thu, Jul 09, 2026 at 01:56:02PM -0500, Nathan Bossart wrote:
> Cool, I'll address everyone's feedback and get this committed in the next
> few days.
Committed.
--
nathan
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-17 18:40 ` surya poondla <suryapoondla4@gmail.com>
2026-06-18 06:44 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2 siblings, 1 reply; 44+ messages in thread
From: surya poondla @ 2026-06-17 18:40 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi ChangAo,
Thank you for reporting and fixing the issue.
The race condition looks real, I confirmed it against the current HEAD.
One thing worth adding to the diagnosis: list construction in
get_all_vacuum_rels() runs in the outer transaction, before vacuum() enters
its PG_TRY block,
so the error from pg_class_aclmask_ext() will abort the entire VACUUM
operation. This is clearly a bug
Updating vacuum_is_permitted_for_relation() to call pg_class_aclcheck_ext()
with is_missing rather than pg_class_aclcheck() looks right to me.
For pg_class_aclcheck(), I checked the other three callers,
expand_vacuum_rel(), vacuum_rel(), and analyze_rel() and each one already
holds a relation lock by the time it reaches this function,
and the error will never fire in those paths.
On Kyotaro's alternative of taking ConditionalLockRelationOid() during list
construction, in the style of get_tables_to_repack().
I agree holding many locks during list build is a real cost on busy
databases, and since each table is processed in its
own transaction, vacuum_open_relation() still has to handle the post-list
drop case regardless.
A couple of points on the patch itself:
1. The bug is racy but the injection_points framework
(src/test/modules/injection_points) can make it deterministic.
We can put an INJECTION_POINT() inside the heap_getnext() loop in
get_all_vacuum_rels() and adding an isolation spec that parks
VACUUM there, runs DROP TABLE in another session, then resumes VACUUM and
asserts it completes with a WARNING.
2. Minor comment vacuum_open_relation() already emits an identically-worded
"relation no longer exists" message with errcode(ERRCODE_UNDEFINED_TABLE).
Worth adding the same errcode to the two new ereports so the SQLSTATE stays
consistent for the same logical event.
This looks like a long-standing bug and I feel it should be backported.
Overall, the patch is heading in the right direction.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-17 18:40 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
@ 2026-06-18 06:44 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-22 21:26 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-18 06:44 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi Surya,
Thanks for the comments!
> A couple of points on the patch itself:
> 1. The bug is racy but the injection_points framework (src/test/modules/injection_points) can make it deterministic.
> We can put an INJECTION_POINT() inside the heap_getnext() loop in get_all_vacuum_rels() and adding an isolation spec that parks
> VACUUM there, runs DROP TABLE in another session, then resumes VACUUM and asserts it completes with a WARNING.
>
> 2. Minor comment vacuum_open_relation() already emits an identically-worded
> "relation no longer exists" message with errcode(ERRCODE_UNDEFINED_TABLE).
> Worth adding the same errcode to the two new ereports so the SQLSTATE stays consistent for the same logical event.
Fixed. Please see the v2 patches.
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v2-0001-Handle-concurrent-drop-when-doing-whole-database-.patch (2.6K, ../../tencent_8F84EE14BED66AA768C854E0E11E14D8850A@qq.com/2-v2-0001-Handle-concurrent-drop-when-doing-whole-database-.patch)
download | inline diff:
From ead91dad977e844f602ddcbb3d49e92775121843 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Thu, 18 Jun 2026 10:49:29 +0800
Subject: [PATCH v2 1/2] Handle concurrent drop when doing whole database
vacuum.
When doing a whole database vacuum, we scan pg_class to construct
a list of vacuumable tables. For each vacuumable table, we call
vacuum_is_permitted_for_relation() to check permissions. If a
concurrent drop happens, the pg_class_aclcheck() might report an
error because of failing to search the syscache.
To fix it, we use pg_class_aclcheck_ext() to detect the concurrent
drop and report a warning instead.
---
src/backend/commands/vacuum.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..1a8e36271b8 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -721,6 +721,7 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
uint32 options)
{
char *relname;
+ bool is_missing = false;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
@@ -733,16 +734,22 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
!reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+ pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
return true;
relname = NameStr(reltuple->relname);
if ((options & VACOPT_VACUUM) != 0)
{
- ereport(WARNING,
- (errmsg("permission denied to vacuum \"%s\", skipping it",
- relname)));
+ if (is_missing)
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("skipping vacuum of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to vacuum \"%s\", skipping it",
+ relname)));
/*
* For VACUUM ANALYZE, both logs could show up, but just generate
@@ -753,9 +760,17 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
}
if ((options & VACOPT_ANALYZE) != 0)
- ereport(WARNING,
- (errmsg("permission denied to analyze \"%s\", skipping it",
- relname)));
+ {
+ if (is_missing)
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("skipping analyze of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to analyze \"%s\", skipping it",
+ relname)));
+ }
return false;
}
--
2.34.1
[application/octet-stream] v2-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch (4.4K, ../../tencent_8F84EE14BED66AA768C854E0E11E14D8850A@qq.com/3-v2-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch)
download | inline diff:
From e2c51899118de0a34529646f3989e9e560fb735a Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Thu, 18 Jun 2026 14:28:57 +0800
Subject: [PATCH v2 2/2] Add test case for vacuum with a concurrent drop.
---
src/backend/commands/vacuum.c | 2 +
src/test/modules/injection_points/Makefile | 1 +
.../expected/vacuum_concurrent_drop.out | 29 ++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/vacuum_concurrent_drop.spec | 54 +++++++++++++++++++
5 files changed, 87 insertions(+)
create mode 100644 src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
create mode 100644 src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 1a8e36271b8..f971d174310 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1062,6 +1062,8 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
scan = table_beginscan_catalog(pgclass, 0, NULL);
+ INJECTION_POINT("vacuum-constructing-vacuumable-rels", NULL);
+
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index c01d2fb095c..2fae0e01f15 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -18,6 +18,7 @@ ISOLATION = basic \
repack_temporal \
repack_temporal_multirange \
repack_toast \
+ vacuum_concurrent_drop \
syscache-update-pruned \
heap_lock_update
diff --git a/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out b/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
new file mode 100644
index 00000000000..eab444a1429
--- /dev/null
+++ b/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
@@ -0,0 +1,29 @@
+Parsed test spec with 2 sessions
+
+starting permutation: setrole1 vacuum1 drop2 wakeup2 resetrole1
+step setrole1:
+ SET ROLE regress_vacuum;
+
+step vacuum1:
+ VACUUM;
+ <waiting ...>
+step drop2:
+ DROP TABLE foo;
+
+step wakeup2:
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step vacuum1: <... completed>
+step resetrole1:
+ RESET ROLE;
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 59dba1cb023..dbb1edf0bfe 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -49,6 +49,7 @@ tests += {
'repack_temporal',
'repack_temporal_multirange',
'repack_toast',
+ 'vacuum_concurrent_drop',
'syscache-update-pruned',
'heap_lock_update',
],
diff --git a/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
new file mode 100644
index 00000000000..6da65fa5e47
--- /dev/null
+++ b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
@@ -0,0 +1,54 @@
+# Test vacuum with a concurrent drop when constructing the list
+# of vacuumable relations. The vacuum should not error out when
+# a concurrent drop happens.
+
+setup
+{
+ CREATE EXTENSION injection_points;
+ CREATE ROLE regress_vacuum;
+ CREATE TABLE foo (id int);
+}
+# DROP TABLE is in 'drop2'
+teardown
+{
+ DROP ROLE regress_vacuum;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('vacuum-constructing-vacuumable-rels', 'wait');
+ SET client_min_messages = ERROR;
+}
+# Make sure that current user is not the owner of current database.
+step setrole1
+{
+ SET ROLE regress_vacuum;
+}
+step vacuum1
+{
+ VACUUM;
+}
+step resetrole1
+{
+ RESET ROLE;
+}
+teardown
+{
+ RESET client_min_messages;
+ SELECT injection_points_detach('vacuum-constructing-vacuumable-rels');
+}
+
+session s2
+step drop2
+{
+ DROP TABLE foo;
+}
+step wakeup2
+{
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+}
+
+permutation setrole1 vacuum1 drop2 wakeup2 resetrole1
--
2.34.1
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-17 18:40 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-18 06:44 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-22 21:26 ` surya poondla <suryapoondla4@gmail.com>
2026-06-23 05:20 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
0 siblings, 1 reply; 44+ messages in thread
From: surya poondla @ 2026-06-22 21:26 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi ChangAo,
Thank you for adding the test case.
I tested v2 locally and looks good.
A few suggestions:
In vacuum.c:
1) The header comment on vacuum_is_permitted_for_relation() still describes
only the permission-check path.
Now that the function also signals "relation no longer exists", the header
should mention both cases.
2) A one-line comment at the pg_class_aclcheck_ext() call explaining why
the _ext variant is needed (concurrent drop in the whole-database
list-construction path) would help future readers, and guard against a
cleanup patch that silently reverts to pg_class_aclcheck().
In vacuum_concurrent_drop.spec:
1) The spec sets "client_min_messages = ERROR", which suppresses the very
WARNING the patch emits.
Let's have the client_min_messages to WARNING, so we see the warning in the
.out file.
2) Minor comment, maybe adding the ANALYZE permutation (or a sibling spec)
would test that code path too. (Not a mandatory thing but a good to have)
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-17 18:40 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-18 06:44 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-22 21:26 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
@ 2026-06-23 05:20 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-24 22:58 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-23 05:20 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi Surya,
> A few suggestions:
> In vacuum.c:
> 1) The header comment on vacuum_is_permitted_for_relation() still describes only the permission-check path.
> Now that the function also signals "relation no longer exists", the header should mention both cases.
> 2) A one-line comment at the pg_class_aclcheck_ext() call explaining why the _ext variant is needed (concurrent drop in the whole-database
> list-construction path) would help future readers, and guard against a cleanup patch that silently reverts to pg_class_aclcheck().
Fixed.
> In vacuum_concurrent_drop.spec:
> 1) The spec sets "client_min_messages = ERROR", which suppresses the very WARNING the patch emits.
> Let's have the client_min_messages to WARNING, so we see the warning in the .out file.
Fixed. We need to grant pg_maintain to regress_vacuum to avoid "permission denied ..."
warnings. For versions (< 17) which don't have pg_maintain role, we may still want to use
"client_min_messages = ERROR".
> 2) Minor comment, maybe adding the ANALYZE permutation (or a sibling spec) would test that code path too. (Not a mandatory thing but a good to have)
Fixed.
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v3-0001-Handle-concurrent-drop-when-doing-whole-database-.patch (3.4K, ../../tencent_AEEC50A342AF5A87145E99373CA2034D3B07@qq.com/2-v3-0001-Handle-concurrent-drop-when-doing-whole-database-.patch)
download | inline diff:
From aaaa35d648b77decc02a0027a387406c7cca97a5 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Tue, 23 Jun 2026 11:12:23 +0800
Subject: [PATCH v3 1/2] Handle concurrent drop when doing whole database
vacuum.
When doing a whole database vacuum, we scan pg_class to construct
a list of vacuumable tables. For each vacuumable table, we call
vacuum_is_permitted_for_relation() to check permissions. If a
concurrent drop happens, the pg_class_aclcheck() might report an
error because of failing to search the syscache.
To fix it, we use pg_class_aclcheck_ext() to detect the concurrent
drop and report a warning instead.
---
src/backend/commands/vacuum.c | 36 ++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..2217188d86e 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -715,12 +715,18 @@ vacuum(List *relations, const VacuumParams *params, BufferAccessStrategy bstrate
* If not, issue a WARNING log message and return false to let the caller
* decide what to do with this relation. This routine is used to decide if a
* relation can be processed for VACUUM or ANALYZE.
+ *
+ * Note that the relation might not be locked, so it can be dropped concurrently.
+ * This can happen when doing a whole database vacuum or analyze in
+ * get_all_vacuum_rels(). We issue a WARNING log message and return false in
+ * this case.
*/
bool
vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
uint32 options)
{
char *relname;
+ bool is_missing = false;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
@@ -729,20 +735,28 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
* following are true:
* - the role owns the current database and the relation is not shared
* - the role has the MAINTAIN privilege on the relation
+ *
+ * Note: use pg_class_aclcheck_ext() to detect a concurrent drop.
*----------
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
!reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+ pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
return true;
relname = NameStr(reltuple->relname);
if ((options & VACOPT_VACUUM) != 0)
{
- ereport(WARNING,
- (errmsg("permission denied to vacuum \"%s\", skipping it",
- relname)));
+ if (is_missing)
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("skipping vacuum of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to vacuum \"%s\", skipping it",
+ relname)));
/*
* For VACUUM ANALYZE, both logs could show up, but just generate
@@ -753,9 +767,17 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
}
if ((options & VACOPT_ANALYZE) != 0)
- ereport(WARNING,
- (errmsg("permission denied to analyze \"%s\", skipping it",
- relname)));
+ {
+ if (is_missing)
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("skipping analyze of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to analyze \"%s\", skipping it",
+ relname)));
+ }
return false;
}
--
2.34.1
[application/octet-stream] v3-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch (5.3K, ../../tencent_AEEC50A342AF5A87145E99373CA2034D3B07@qq.com/3-v3-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch)
download | inline diff:
From ef1fdbef72f8ac090f01b6d8432d6fb4f8971bee Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Tue, 23 Jun 2026 13:04:11 +0800
Subject: [PATCH v3 2/2] Add test case for vacuum with a concurrent drop.
---
src/backend/commands/vacuum.c | 2 +
src/test/modules/injection_points/Makefile | 1 +
.../expected/vacuum_concurrent_drop.out | 65 +++++++++++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/vacuum_concurrent_drop.spec | 64 ++++++++++++++++++
5 files changed, 133 insertions(+)
create mode 100644 src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
create mode 100644 src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2217188d86e..d895b31be6b 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1069,6 +1069,8 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
scan = table_beginscan_catalog(pgclass, 0, NULL);
+ INJECTION_POINT("vacuum-constructing-vacuumable-rels", NULL);
+
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index c01d2fb095c..2fae0e01f15 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -18,6 +18,7 @@ ISOLATION = basic \
repack_temporal \
repack_temporal_multirange \
repack_toast \
+ vacuum_concurrent_drop \
syscache-update-pruned \
heap_lock_update
diff --git a/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out b/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
new file mode 100644
index 00000000000..c46b267a27f
--- /dev/null
+++ b/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
@@ -0,0 +1,65 @@
+Parsed test spec with 2 sessions
+
+starting permutation: create1 setrole1 vacuum1 drop2 wakeup2 resetrole1
+step create1:
+ CREATE TABLE foo (id int);
+
+step setrole1:
+ SET ROLE regress_vacuum;
+
+step vacuum1:
+ VACUUM;
+ <waiting ...>
+step drop2:
+ DROP TABLE foo;
+
+step wakeup2:
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+s1: WARNING: skipping vacuum of "foo" --- relation no longer exists
+step vacuum1: <... completed>
+step resetrole1:
+ RESET ROLE;
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
+
+starting permutation: create1 setrole1 analyze1 drop2 wakeup2 resetrole1
+step create1:
+ CREATE TABLE foo (id int);
+
+step setrole1:
+ SET ROLE regress_vacuum;
+
+step analyze1:
+ ANALYZE;
+ <waiting ...>
+step drop2:
+ DROP TABLE foo;
+
+step wakeup2:
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+s1: WARNING: skipping analyze of "foo" --- relation no longer exists
+step analyze1: <... completed>
+step resetrole1:
+ RESET ROLE;
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 59dba1cb023..dbb1edf0bfe 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -49,6 +49,7 @@ tests += {
'repack_temporal',
'repack_temporal_multirange',
'repack_toast',
+ 'vacuum_concurrent_drop',
'syscache-update-pruned',
'heap_lock_update',
],
diff --git a/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
new file mode 100644
index 00000000000..793a12762ae
--- /dev/null
+++ b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
@@ -0,0 +1,64 @@
+# Test vacuum/analyze with a concurrent drop when constructing the list
+# of vacuumable relations. The vacuum/analyze should not error out when
+# a concurrent drop happens.
+
+setup
+{
+ CREATE EXTENSION injection_points;
+ CREATE ROLE regress_vacuum IN ROLE pg_maintain;
+}
+teardown
+{
+ DROP ROLE regress_vacuum;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('vacuum-constructing-vacuumable-rels', 'wait');
+ SET client_min_messages = WARNING;
+}
+step create1
+{
+ CREATE TABLE foo (id int);
+}
+# Make sure that current user is not the owner of current database.
+step setrole1
+{
+ SET ROLE regress_vacuum;
+}
+step vacuum1
+{
+ VACUUM;
+}
+step analyze1
+{
+ ANALYZE;
+}
+step resetrole1
+{
+ RESET ROLE;
+}
+teardown
+{
+ RESET client_min_messages;
+ SELECT injection_points_detach('vacuum-constructing-vacuumable-rels');
+}
+
+session s2
+step drop2
+{
+ DROP TABLE foo;
+}
+step wakeup2
+{
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+}
+
+# Test vacuum
+permutation create1 setrole1 vacuum1 drop2 wakeup2 resetrole1
+
+# Test analyze
+permutation create1 setrole1 analyze1 drop2 wakeup2 resetrole1
--
2.34.1
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-17 18:40 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-18 06:44 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-22 21:26 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-23 05:20 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-24 22:58 ` surya poondla <suryapoondla4@gmail.com>
0 siblings, 0 replies; 44+ messages in thread
From: surya poondla @ 2026-06-24 22:58 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi ChangAo,
Thank you for incorporating the changes.
I applied and tested the v3 patch, it works all good.
Both permutations (vacuum and analyze) pass and the expected output now
asserts 's1: WARNING: skipping vacuum of "foo" and' 's1: WARNING:
skipping analyze of "foo" ' respectively which looks correct.
Regarding the backpatch, I agree this is a long standing bug, the test spec
might need to change as there is no pg_maintain, but the fix itself should
apply cleanly.
Overall, the patch looks in good shape.
Thank you for working on this issue.
You can create a commitfest entry.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-23 21:06 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-23 21:06 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Sun, Jun 14, 2026 at 12:13 AM cca5507 <cca5507@qq.com> wrote:
>
> Hi hackers,
>
> When doing a whole database vacuum, we scan pg_class to construct
> a list of vacuumable tables. For each vacuumable table, we call
> vacuum_is_permitted_for_relation() to check permissions. If a
> concurrent drop happens, the pg_class_aclcheck() might report an
> error because of failing to search the syscache:
>
> ERROR: relation with OID ****** does not exist
>
> To fix it, we can use pg_class_aclcheck_ext() to detect the concurrent
> drop and report a warning instead.
>
> Note that a concurrent drop after constructing the list of vacuumable
> tables is handled by vacuum_open_relation().
>
> Thoughts?
Nice catch!
IMHO, the vacuum command failing in this situation may not be a huge
problem in practice, because the user sees the error and can re-issue
the command.
get_all_vacuum_rels is called only for database-wide vacuum (neither
autovacuum nor vacuum <<tables-list>> calls it - in those cases, the
permission check happens after the table lock is taken in
vacuum_open_relation).
Let's think about what happens if we don't fix this. For example, a
database-wide vacuum with 1000 tables has scanned 990 of them in
pg_class and the 991st table gets concurrently dropped - the vacuum
command fails and the user needs to notice that and re-issue the
command, at which point it rescans pg_class. What exactly is the
problem we're solving? Is it the vacuum command failing and the user
missing it or forgetting to re-issue? The pg_class scan being
expensive? Or consistency with how autovacuum and vacuum
<<tables-list>> handle this?
ReindexMultipleTables has a similar scan on pg_class with a permission
check, but it is not affected because the permission check is gated
behind shared relations (so droppable user tables never reach it).
Some comments on the v3 patches:
1/
+ *
+ * Note: use pg_class_aclcheck_ext() to detect a concurrent drop.
This comment seems redundant - the function call, the missing_ok
check, and the warning message already make it clear.
2/
+ * Note that the relation might not be locked, so it can be dropped
concurrently.
+ * This can happen when doing a whole database vacuum or analyze in
+ * get_all_vacuum_rels(). We issue a WARNING log message and return false in
+ * this case.
Instead of saying "relation might not be locked", can we be more
explicit about the exact cases? For example: For database-wide
vacuums, this function is called without holding a relation lock, so
the table can be dropped concurrently. In that case, issue a WARNING
and return false.
I couldn't think of a good way to reproduce this other than creating a
large number of tables to make the pg_class scan costlier. For
predictable testing I might agree to adding an injection point.
However, I have some comments:
3/
+# Make sure that current user is not the owner of current database.
+step setrole1
+{
+ SET ROLE regress_vacuum;
+}
Is this necessary?
4/
+step analyze1
+{
+ ANALYZE;
+}
I think just testing VACUUM or VACUUM ANALYZE in a single test keeps
things smaller, since the underlying code is the same for both.
5/
+++ b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
Why do you need isolation tests with an injection point? Why not a TAP
test under src/test/modules/test_misc? A TAP test with just VACUUM
(not ANALYZE) would keep the test simpler with fewer lines of code.
6/
+ INJECTION_POINT("vacuum-constructing-vacuumable-rels", NULL);
Nit: How about a shorter name like vacuum-get-rels or vacuum-build-rels?
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-25 05:53 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-25 05:53 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; surya poondla <suryapoondla4@gmail.com>; Michael Paquier <michael@paquier.xyz>
Hi,
> Some comments on the v3 patches:
>
> 1/
> + *
> + * Note: use pg_class_aclcheck_ext() to detect a concurrent drop.
>
> This comment seems redundant - the function call, the missing_ok
> check, and the warning message already make it clear.
Yeah, the code can explain itself clearly, so removed.
> 2/
> + * Note that the relation might not be locked, so it can be dropped
> concurrently.
> + * This can happen when doing a whole database vacuum or analyze in
> + * get_all_vacuum_rels(). We issue a WARNING log message and return false in
> + * this case.
>
> Instead of saying "relation might not be locked", can we be more
> explicit about the exact cases? For example: For database-wide
> vacuums, this function is called without holding a relation lock, so
> the table can be dropped concurrently. In that case, issue a WARNING
> and return false.
Fixed.
> I couldn't think of a good way to reproduce this other than creating a
> large number of tables to make the pg_class scan costlier. For
> predictable testing I might agree to adding an injection point.
> However, I have some comments:
>
> 3/
> +# Make sure that current user is not the owner of current database.
> +step setrole1
> +{
> + SET ROLE regress_vacuum;
> +}
>
> Is this necessary?
Yes, we won't check MAINTAIN privilege on the table if current user is the
owner of current database.
> 4/
> +step analyze1
> +{
> + ANALYZE;
> +}
>
> I think just testing VACUUM or VACUUM ANALYZE in a single test keeps
> things smaller, since the underlying code is the same for both.
Yeah, the ANALYZE might also increase the test time, so removed.
> 5/
> +++ b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
>
> Why do you need isolation tests with an injection point? Why not a TAP
> test under src/test/modules/test_misc? A TAP test with just VACUUM
> (not ANALYZE) would keep the test simpler with fewer lines of code.
The src/test/perl/README says:
It's used to drive tests for backup and restore, replication, etc - anything that can't
really be expressed using pg_regress or the isolation test framework.
I can write a TAP test if we decide to use it.
> 6/
> + INJECTION_POINT("vacuum-constructing-vacuumable-rels", NULL);
>
> Nit: How about a shorter name like vacuum-get-rels or vacuum-build-rels?
I think the current one is ok, so I didn't change this.
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v4-0001-Handle-concurrent-drop-when-doing-whole-database-.patch (3.1K, ../../tencent_67C6166FFDE0CA2E25F4AC90A97621454605@qq.com/2-v4-0001-Handle-concurrent-drop-when-doing-whole-database-.patch)
download | inline diff:
From acfa320c1834ed0cd924fe8250d309905ae398f4 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Thu, 25 Jun 2026 13:28:14 +0800
Subject: [PATCH v4 1/2] Handle concurrent drop when doing whole database
vacuum.
When doing a whole database vacuum, we scan pg_class to construct
a list of vacuumable tables. For each vacuumable table, we call
vacuum_is_permitted_for_relation() to check permissions. If a
concurrent drop happens, the pg_class_aclcheck() might report an
error because of failing to search the syscache.
To fix it, we use pg_class_aclcheck_ext() to detect the concurrent
drop and report a warning instead.
---
src/backend/commands/vacuum.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..79d09b9d70f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -715,12 +715,17 @@ vacuum(List *relations, const VacuumParams *params, BufferAccessStrategy bstrate
* If not, issue a WARNING log message and return false to let the caller
* decide what to do with this relation. This routine is used to decide if a
* relation can be processed for VACUUM or ANALYZE.
+ *
+ * Note: this function is called without holding a relation lock for database-wide
+ * VACUUM or ANALYZE, so the relation can be dropped concurrently. In that
+ * case, issue a WARNING and return false.
*/
bool
vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
uint32 options)
{
char *relname;
+ bool is_missing = false;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
@@ -733,16 +738,22 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
!reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+ pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
return true;
relname = NameStr(reltuple->relname);
if ((options & VACOPT_VACUUM) != 0)
{
- ereport(WARNING,
- (errmsg("permission denied to vacuum \"%s\", skipping it",
- relname)));
+ if (is_missing)
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("skipping vacuum of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to vacuum \"%s\", skipping it",
+ relname)));
/*
* For VACUUM ANALYZE, both logs could show up, but just generate
@@ -753,9 +764,17 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
}
if ((options & VACOPT_ANALYZE) != 0)
- ereport(WARNING,
- (errmsg("permission denied to analyze \"%s\", skipping it",
- relname)));
+ {
+ if (is_missing)
+ ereport(WARNING,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("skipping analyze of \"%s\" --- relation no longer exists",
+ relname)));
+ else
+ ereport(WARNING,
+ (errmsg("permission denied to analyze \"%s\", skipping it",
+ relname)));
+ }
return false;
}
--
2.34.1
[application/octet-stream] v4-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch (4.6K, ../../tencent_67C6166FFDE0CA2E25F4AC90A97621454605@qq.com/3-v4-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch)
download | inline diff:
From d3df4189fcb6653f81efa55db77982ade5752774 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Thu, 25 Jun 2026 13:47:41 +0800
Subject: [PATCH v4 2/2] Add test case for vacuum with a concurrent drop.
---
src/backend/commands/vacuum.c | 2 +
src/test/modules/injection_points/Makefile | 1 +
.../expected/vacuum_concurrent_drop.out | 38 +++++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/vacuum_concurrent_drop.spec | 55 +++++++++++++++++++
5 files changed, 97 insertions(+)
create mode 100644 src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
create mode 100644 src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 79d09b9d70f..12d4ec11167 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1066,6 +1066,8 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
scan = table_beginscan_catalog(pgclass, 0, NULL);
+ INJECTION_POINT("vacuum-constructing-vacuumable-rels", NULL);
+
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index c01d2fb095c..2fae0e01f15 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -18,6 +18,7 @@ ISOLATION = basic \
repack_temporal \
repack_temporal_multirange \
repack_toast \
+ vacuum_concurrent_drop \
syscache-update-pruned \
heap_lock_update
diff --git a/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out b/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
new file mode 100644
index 00000000000..40172f42f1a
--- /dev/null
+++ b/src/test/modules/injection_points/expected/vacuum_concurrent_drop.out
@@ -0,0 +1,38 @@
+Parsed test spec with 2 sessions
+
+starting permutation: create1 setrole1 vacuum1 drop2 wakeup2 resetrole1
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step create1:
+ CREATE TABLE foo (id int);
+
+step setrole1:
+ SET ROLE regress_vacuum;
+
+step vacuum1:
+ VACUUM;
+ <waiting ...>
+step drop2:
+ DROP TABLE foo;
+
+step wakeup2:
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+s1: WARNING: skipping vacuum of "foo" --- relation no longer exists
+step vacuum1: <... completed>
+step resetrole1:
+ RESET ROLE;
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 59dba1cb023..dbb1edf0bfe 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -49,6 +49,7 @@ tests += {
'repack_temporal',
'repack_temporal_multirange',
'repack_toast',
+ 'vacuum_concurrent_drop',
'syscache-update-pruned',
'heap_lock_update',
],
diff --git a/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
new file mode 100644
index 00000000000..60c7d14d8c7
--- /dev/null
+++ b/src/test/modules/injection_points/specs/vacuum_concurrent_drop.spec
@@ -0,0 +1,55 @@
+# Test vacuum with a concurrent drop when constructing the list
+# of vacuumable relations. The vacuum should not error out when
+# a concurrent drop happens.
+
+setup
+{
+ CREATE EXTENSION injection_points;
+ CREATE ROLE regress_vacuum IN ROLE pg_maintain;
+}
+teardown
+{
+ DROP ROLE regress_vacuum;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('vacuum-constructing-vacuumable-rels', 'wait');
+}
+step create1
+{
+ CREATE TABLE foo (id int);
+}
+# Make sure that current user is not the owner of current database so
+# that we will check the MAINTAIN privilege on 'foo'.
+step setrole1
+{
+ SET ROLE regress_vacuum;
+}
+step vacuum1
+{
+ VACUUM;
+}
+step resetrole1
+{
+ RESET ROLE;
+}
+teardown
+{
+ SELECT injection_points_detach('vacuum-constructing-vacuumable-rels');
+}
+
+session s2
+step drop2
+{
+ DROP TABLE foo;
+}
+step wakeup2
+{
+ SELECT injection_points_wakeup('vacuum-constructing-vacuumable-rels');
+}
+
+permutation create1 setrole1 vacuum1 drop2 wakeup2 resetrole1
--
2.34.1
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-25 06:19 ` Michael Paquier <michael@paquier.xyz>
2026-06-25 16:51 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 2 replies; 44+ messages in thread
From: Michael Paquier @ 2026-06-25 06:19 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; surya poondla <suryapoondla4@gmail.com>
On Thu, Jun 25, 2026 at 01:53:39PM +0800, cca5507 wrote:
> I think the current one is ok, so I didn't change this.
+ * Note: this function is called without holding a relation lock for database-wide
+ * VACUUM or ANALYZE, so the relation can be dropped concurrently. In that
+ * case, issue a WARNING and return false.
FWIW, even after sleeping on it, I don't think that this patch is
going in the right direction. I am pretty sure that we should just
lift the ACL check in get_all_vacuum_rels() and always require
vacuum_is_permitted_for_relation() to have the relation locked when
called. This way we would rely on one single code path for the ACL
check, even if it means holding a reference of a relation OID for
something to be processsed later. So I would revisit the choice made
in a556549d7e6d. The isolation test vacuum-conflict also seems OK
with this shortcut, after a quick test, which was the kind of patterns
this commit is after.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../ajzIjd8sb2wiIXs5@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-25 16:51 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 17:41 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
1 sibling, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-25 16:51 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; surya poondla <suryapoondla4@gmail.com>
Hi,
On Wed, Jun 24, 2026 at 11:20 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Jun 25, 2026 at 01:53:39PM +0800, cca5507 wrote:
> > I think the current one is ok, so I didn't change this.
>
> + * Note: this function is called without holding a relation lock for database-wide
> + * VACUUM or ANALYZE, so the relation can be dropped concurrently. In that
> + * case, issue a WARNING and return false.
>
> FWIW, even after sleeping on it, I don't think that this patch is
> going in the right direction. I am pretty sure that we should just
> lift the ACL check in get_all_vacuum_rels() and always require
> vacuum_is_permitted_for_relation() to have the relation locked when
> called. This way we would rely on one single code path for the ACL
> check, even if it means holding a reference of a relation OID for
> something to be processsed later. So I would revisit the choice made
> in a556549d7e6d.
After thinking about it again, I'd take back my concern about the
"unnecessary work" if the ACL check is removed from
get_all_vacuum_rels() and left to vacuum_rel()'s
vacuum_is_permitted_for_relation() after the table lock. My concern
was around a rare use-case, and the other vacuum invocations (vacuum
<table-list> and autovacuum) didn't have this problem anyway.
I now fully agree with your point on making it consistent across all
vacuum invocations (database-wide, with table list, and autovacuum).
In this case, the 0001 patch could just remove the
vacuum_is_permitted_for_relation call from get_all_vacuum_rels, with a
comment noting that permission checks are done in vacuum_rel.
> The isolation test vacuum-conflict also seems OK
> with this shortcut, after a quick test, which was the kind of patterns
> this commit is after.
One comment on the isolation test - why an isolation test? Why not a
TAP test under src/test/modules/test_misc? IMO, TAP test is easier to
maintain (no expected output file) and fewer LoC. We may not need a
new test file either - the closest place to add it would be
006_signal_autovacuum.pl or add a 100_bugs.pl or such.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-25 16:51 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-25 17:41 ` surya poondla <suryapoondla4@gmail.com>
2026-06-25 18:04 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: surya poondla @ 2026-06-25 17:41 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: cca5507 <cca5507@qq.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi Michael, Bharath, ChangAo,
I tested v3 locally previously, but after thinking through Michael's
argument
> FWIW, even after sleeping on it, I don't think that this patch is
> > going in the right direction. I am pretty sure that we should just
> > lift the ACL check in get_all_vacuum_rels() and always require
> > vacuum_is_permitted_for_relation() to have the relation locked when
> > called. This way we would rely on one single code path for the ACL
> > check, even if it means holding a reference of a relation OID for
> > something to be processsed later. So I would revisit the choice made
> > in a556549d7e6d.
>
I agree the architectural direction is better.
Removing the early ACL check from get_all_vacuum_rels() means the syscache
lookup that was racing with concurrent DROP never happens at list
construction
at all. The race altogether vanishes and we don't need to handle the
is_missing.
Also it brings the database-wide VACUUM path in line with VACUUM
<table-list> and autovacuum.
On the partial MAINTAIN case, the per-relation transaction cost is real,
and the rejected vacuum_rel calls would still take ShareUpdateExclusiveLock
briefly, which
could contend with autovacuum on tables the user can't vacuum anyway.
The architectural simplification is durable. Seems like the right tradeoff
as this partial MAINTAIN configuration is not widely used.
On the test front: once the race code path is gone, the injection point
with the spec file isn't strictly needed either.
A simpler test that shows "VACUUM with a concurrent drop successfully
completes without error" should suffice.
Bharath's TAP test suggestion under src/test/modules/test_misc seems like a
good fit now.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-25 16:51 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 17:41 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
@ 2026-06-25 18:04 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 22:23 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-25 18:04 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; +Cc: cca5507 <cca5507@qq.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Thu, Jun 25, 2026 at 10:41 AM surya poondla <suryapoondla4@gmail.com> wrote:
>
> On the test front: once the race code path is gone, the injection point with the spec file isn't strictly needed either.
> A simpler test that shows "VACUUM with a concurrent drop successfully completes without error" should suffice.
> Bharath's TAP test suggestion under src/test/modules/test_misc seems like a good fit now.
Thinking about this further, the test is NOT needed at all with this
approach because vacuum_open_relation() already protects against
concurrent drops. So, -1 for adding a new test.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-25 16:51 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 17:41 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-25 18:04 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-25 22:23 ` Michael Paquier <michael@paquier.xyz>
2026-06-26 01:39 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Michael Paquier @ 2026-06-25 22:23 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: surya poondla <suryapoondla4@gmail.com>; cca5507 <cca5507@qq.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Jun 25, 2026 at 11:04:55AM -0700, Bharath Rupireddy wrote:
> On Thu, Jun 25, 2026 at 10:41 AM surya poondla <suryapoondla4@gmail.com> wrote:
>>
>> On the test front: once the race code path is gone, the injection point with the spec file isn't strictly needed either.
>> A simpler test that shows "VACUUM with a concurrent drop successfully completes without error" should suffice.
>> Bharath's TAP test suggestion under src/test/modules/test_misc seems like a good fit now.
>
> Thinking about this further, the test is NOT needed at all with this
> approach because vacuum_open_relation() already protects against
> concurrent drops. So, -1 for adding a new test.
Well, as you mentioned upthread the only test infrastructure that we
could use here is TAP, as database-wide VACUUMs could get costly when
running other tests in parallel and we need stability. But I cannot
get excited about the cost this would have versus the coverage we get.
Cycles are better spent elsewhere, even if injection points could be
leveraged here.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aj2qamkSPpNkgNwq@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-25 16:51 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 17:41 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-25 18:04 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 22:23 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-26 01:39 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 0 replies; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-26 01:39 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: surya poondla <suryapoondla4@gmail.com>; cca5507 <cca5507@qq.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Thu, Jun 25, 2026 at 3:23 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Jun 25, 2026 at 11:04:55AM -0700, Bharath Rupireddy wrote:
> > On Thu, Jun 25, 2026 at 10:41 AM surya poondla <suryapoondla4@gmail.com> wrote:
> >>
> >> On the test front: once the race code path is gone, the injection point with the spec file isn't strictly needed either.
> >> A simpler test that shows "VACUUM with a concurrent drop successfully completes without error" should suffice.
> >> Bharath's TAP test suggestion under src/test/modules/test_misc seems like a good fit now.
> >
> > Thinking about this further, the test is NOT needed at all with this
> > approach because vacuum_open_relation() already protects against
> > concurrent drops. So, -1 for adding a new test.
>
> Well, as you mentioned upthread the only test infrastructure that we
> could use here is TAP, as database-wide VACUUMs could get costly when
> running other tests in parallel and we need stability. But I cannot
> get excited about the cost this would have versus the coverage we get.
> Cycles are better spent elsewhere, even if injection points could be
> leveraged here.
Agreed. I don't think it brings in any new coverage. Let's not
complicate things with injection points and tests for this fix.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-26 02:31 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
1 sibling, 1 reply; 44+ messages in thread
From: Kyotaro Horiguchi @ 2026-06-26 02:31 UTC (permalink / raw)
To: michael@paquier.xyz; +Cc: cca5507@qq.com; bharath.rupireddyforpostgres@gmail.com; pgsql-hackers@lists.postgresql.org; suryapoondla4@gmail.com
At Thu, 25 Jun 2026 15:19:57 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> FWIW, even after sleeping on it, I don't think that this patch is
> going in the right direction. I am pretty sure that we should just
> lift the ACL check in get_all_vacuum_rels() and always require
> vacuum_is_permitted_for_relation() to have the relation locked when
> called. This way we would rely on one single code path for the ACL
> check, even if it means holding a reference of a relation OID for
> something to be processsed later. So I would revisit the choice made
FWIW, I agree with this direction.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2026-06-26 09:36 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-26 09:36 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; michael <michael@paquier.xyz>; +Cc: bharath.rupireddyforpostgres <bharath.rupireddyforpostgres@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
> > FWIW, even after sleeping on it, I don't think that this patch is
> > going in the right direction. I am pretty sure that we should just
> > lift the ACL check in get_all_vacuum_rels() and always require
> > vacuum_is_permitted_for_relation() to have the relation locked when
> > called. This way we would rely on one single code path for the ACL
> > check, even if it means holding a reference of a relation OID for
> > something to be processsed later. So I would revisit the choice made
>
> FWIW, I agree with this direction.
This direction is also OK for me. We might want to fix get_tables_to_repack()
together?
--
Regards,
ChangAo Chen
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-26 21:19 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 02:09 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
0 siblings, 2 replies; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-26 21:19 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; michael <michael@paquier.xyz>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
On Fri, Jun 26, 2026 at 2:36 AM cca5507 <cca5507@qq.com> wrote:
>
> Hi,
>
> > > FWIW, even after sleeping on it, I don't think that this patch is
> > > going in the right direction. I am pretty sure that we should just
> > > lift the ACL check in get_all_vacuum_rels() and always require
> > > vacuum_is_permitted_for_relation() to have the relation locked when
> > > called. This way we would rely on one single code path for the ACL
> > > check, even if it means holding a reference of a relation OID for
> > > something to be processsed later. So I would revisit the choice made
> >
> > FWIW, I agree with this direction.
>
> This direction is also OK for me. We might want to fix get_tables_to_repack()
> together?
We want to get the vacuum behavior consistent across all three modes -
database-wide vacuum, vacuum <tables-list>, and autovacuum - as far as
the permissions check is concerned. That is, do the permissions check
after acquiring the table-level lock. This also solves the ERRORs
reported due to concurrent table drops during the permissions check,
which happen because pg_class_aclcheck is used instead of
pg_class_aclcheck_ext with is_missing.
I had a quick look at the repack code and it seems like it can also
run into the same problem because repack_is_permitted_for_relation
uses the same pg_class_aclcheck while building the tables list without
holding locks, and later it rechecks permissions after the table locks
in cluster_rel_recheck anyway. A simple fix there would be to just use
pg_class_aclcheck_ext in repack_is_permitted_for_relation. I recommend
discussing this in a separate thread CC-ing the repack authors to get
agreement.
Do you mind sharing the vacuum-related fix where we have agreement in
this thread? Thank you!
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-27 02:09 ` Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 44+ messages in thread
From: Michael Paquier @ 2026-06-27 02:09 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
On Fri, Jun 26, 2026 at 02:19:37PM -0700, Bharath Rupireddy wrote:
> I had a quick look at the repack code and it seems like it can also
> run into the same problem because repack_is_permitted_for_relation
> uses the same pg_class_aclcheck while building the tables list without
> holding locks, and later it rechecks permissions after the table locks
> in cluster_rel_recheck anyway. A simple fix there would be to just use
> pg_class_aclcheck_ext in repack_is_permitted_for_relation. I recommend
> discussing this in a separate thread CC-ing the repack authors to get
> agreement.
A separate would be appropriate, yes. For the repack part, there may
be an argument for tweaking the behavior (or not). The vacuum part
dates back to 2018, and is much older.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aj8w9b4OOBzyFkhx@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-27 08:19 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
1 sibling, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-27 08:19 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; michael <michael@paquier.xyz>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
> I had a quick look at the repack code and it seems like it can also
> run into the same problem because repack_is_permitted_for_relation
> uses the same pg_class_aclcheck while building the tables list without
> holding locks, and later it rechecks permissions after the table locks
> in cluster_rel_recheck anyway. A simple fix there would be to just use
> pg_class_aclcheck_ext in repack_is_permitted_for_relation. I recommend
> discussing this in a separate thread CC-ing the repack authors to get
> agreement.
Note that we call ConditionalLockRelationOid() in get_tables_to_repack(),
which is an unexpected behavior I think. I have reported this issue in this
thread:
https://www.postgresql.org/message-id/flat/tencent_9F290B256A3F52B66542F1140E32ECC64309%40qq.com
> Do you mind sharing the vacuum-related fix where we have agreement in
> this thread? Thank you!
Please see the v5 patch.
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v5-0001-Do-not-check-permissions-in-get_all_vacuum_rels.patch (1.8K, ../../tencent_515EB50C2919321578C207247632A1218508@qq.com/2-v5-0001-Do-not-check-permissions-in-get_all_vacuum_rels.patch)
download | inline diff:
From 56da12f8f5d4c6b1deee60aa209722ccc57a18b1 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Sat, 27 Jun 2026 15:44:53 +0800
Subject: [PATCH v5] Do not check permissions in get_all_vacuum_rels.
When doing a database-wide vacuum, we scan pg_class to build a list
of vacuumable relations without locking them. The permissions check
might report an error if a concurrent drop happens because of failing
to search the syscache. This commit remove the permissions check in
get_all_vacuum_rels and the permissions check will happen later when
we process each relation. This also makes the behavior consistent
across database-wide vacuum, vacuum <tables-list> and autovacuum.
---
src/backend/commands/vacuum.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..d5ff7dcdb11 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -715,6 +715,8 @@ vacuum(List *relations, const VacuumParams *params, BufferAccessStrategy bstrate
* If not, issue a WARNING log message and return false to let the caller
* decide what to do with this relation. This routine is used to decide if a
* relation can be processed for VACUUM or ANALYZE.
+ *
+ * Note: the relation must be locked to prevent a concurrent drop.
*/
bool
vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
@@ -1068,10 +1070,6 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
!isTempOrTempToastNamespace(classForm->relnamespace))
continue;
- /* check permissions of relation */
- if (!vacuum_is_permitted_for_relation(relid, classForm, options))
- continue;
-
/*
* Build VacuumRelation(s) specifying the table OIDs to be processed.
* We omit a RangeVar since it wouldn't be appropriate to complain
--
2.54.0
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-27 19:32 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-27 19:32 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; michael <michael@paquier.xyz>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
On Sat, Jun 27, 2026 at 1:19 AM cca5507 <cca5507@qq.com> wrote:
>
> > Do you mind sharing the vacuum-related fix where we have agreement in
> > this thread? Thank you!
>
> Please see the v5 patch.
LGTM.
+ *
+ * Note: the relation must be locked to prevent a concurrent drop.
*/
I would have considered adding an
Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true))
instead of the note, but that seems like overkill to me. So, I'm fine
without it.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-29 00:13 ` Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Michael Paquier @ 2026-06-29 00:13 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
On Sat, Jun 27, 2026 at 12:32:42PM -0700, Bharath Rupireddy wrote:
> I would have considered adding an
> Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true))
> instead of the note, but that seems like overkill to me. So, I'm fine
> without it.
An assertion is impossible to miss, acts as self-documentation and it
is not a performance critical path. A note at the top of the function
could be easily missed. I don't see why the note needs to mention the
case of a concurrent drop. So let's just keep the runtime check, then
drop the note, finally call it a day.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../akG4r-_eRtZ5pMGe@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-06-29 00:28 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
0 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-29 00:28 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
On Sun, Jun 28, 2026 at 5:13 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, Jun 27, 2026 at 12:32:42PM -0700, Bharath Rupireddy wrote:
> > I would have considered adding an
> > Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true))
> > instead of the note, but that seems like overkill to me. So, I'm fine
> > without it.
>
> An assertion is impossible to miss, acts as self-documentation and it
> is not a performance critical path. A note at the top of the function
> could be easily missed. I don't see why the note needs to mention the
> case of a concurrent drop. So let's just keep the runtime check, then
> drop the note, finally call it a day.
Works for me. Thanks.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-29 04:37 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 05:01 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
0 siblings, 2 replies; 44+ messages in thread
From: cca5507 @ 2026-06-29 04:37 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
> > > I would have considered adding an
> > > Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true))
> > > instead of the note, but that seems like overkill to me. So, I'm fine
> > > without it.
> >
> > An assertion is impossible to miss, acts as self-documentation and it
> > is not a performance critical path. A note at the top of the function
> > could be easily missed. I don't see why the note needs to mention the
> > case of a concurrent drop. So let's just keep the runtime check, then
> > drop the note, finally call it a day.
>
> Works for me. Thanks.
Fixed.
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v6-0001-Do-not-check-permissions-in-get_all_vacuum_rels.patch (1.7K, ../../tencent_AD5D52285621586D53CA904C2C9EDA81BA06@qq.com/2-v6-0001-Do-not-check-permissions-in-get_all_vacuum_rels.patch)
download | inline diff:
From 5cdbe18a3bc793ea97162a944d44cafd6dc2abea Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Sat, 27 Jun 2026 15:44:53 +0800
Subject: [PATCH v6] Do not check permissions in get_all_vacuum_rels.
When doing a database-wide vacuum, we scan pg_class to build a list
of vacuumable relations without locking them. The permissions check
might report an error if a concurrent drop happens because of failing
to search the syscache. This commit remove the permissions check in
get_all_vacuum_rels and the permissions check will happen later when
we process each relation. This also makes the behavior consistent
across database-wide vacuum, vacuum <tables-list> and autovacuum.
---
src/backend/commands/vacuum.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..7d364f3bc21 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -723,6 +723,7 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
char *relname;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
+ Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true));
/*----------
* A role has privileges to vacuum or analyze the relation if any of the
@@ -1068,10 +1069,6 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
!isTempOrTempToastNamespace(classForm->relnamespace))
continue;
- /* check permissions of relation */
- if (!vacuum_is_permitted_for_relation(relid, classForm, options))
- continue;
-
/*
* Build VacuumRelation(s) specifying the table OIDs to be processed.
* We omit a RangeVar since it wouldn't be appropriate to complain
--
2.54.0
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-29 04:58 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
1 sibling, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-06-29 04:58 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
> > > > I would have considered adding an
> > > > Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true))
> > > > instead of the note, but that seems like overkill to me. So, I'm fine
> > > > without it.
> > >
> > > An assertion is impossible to miss, acts as self-documentation and it
> > > is not a performance critical path. A note at the top of the function
> > > could be easily missed. I don't see why the note needs to mention the
> > > case of a concurrent drop. So let's just keep the runtime check, then
> > > drop the note, finally call it a day.
> >
> > Works for me. Thanks.
>
> Fixed.
Remove some outdated comments in v7.
--
Regards,
ChangAo Chen
Attachments:
[application/octet-stream] v7-0001-Do-not-check-permissions-in-get_all_vacuum_rels.patch (3.3K, ../../tencent_ADEC9C42E70B236A9B521665B2F1380E3409@qq.com/2-v7-0001-Do-not-check-permissions-in-get_all_vacuum_rels.patch)
download | inline diff:
From 08f91f3d7554bc482d615230ad29ae096a8029d7 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Sat, 27 Jun 2026 15:44:53 +0800
Subject: [PATCH v7] Do not check permissions in get_all_vacuum_rels.
When doing a database-wide vacuum, we scan pg_class to build a list
of vacuumable relations without locking them. The permissions check
might report an error if a concurrent drop happens because of failing
to search the syscache. This commit remove the permissions check in
get_all_vacuum_rels and the permissions check will happen later when
we process each relation. This also makes the behavior consistent
across database-wide vacuum, vacuum <tables-list> and autovacuum.
---
src/backend/commands/analyze.c | 7 ++-----
src/backend/commands/vacuum.c | 12 +++---------
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index f66e80b757c..f303c3eed7d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -149,11 +149,8 @@ analyze_rel(Oid relid, RangeVar *relation,
return;
/*
- * Check if relation needs to be skipped based on privileges. This check
- * happens also when building the relation list to analyze for a manual
- * operation, and needs to be done additionally here as ANALYZE could
- * happen across multiple transactions where privileges could have changed
- * in-between. Make sure to generate only logs for ANALYZE in this case.
+ * Check if relation needs to be skipped based on privileges. Make sure
+ * to generate only logs for ANALYZE in this case.
*/
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
onerel->rd_rel,
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..59d7640a6db 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -723,6 +723,7 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
char *relname;
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
+ Assert(CheckRelationOidLockedByMe(relid, AccessShareLock, true));
/*----------
* A role has privileges to vacuum or analyze the relation if any of the
@@ -1068,10 +1069,6 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
!isTempOrTempToastNamespace(classForm->relnamespace))
continue;
- /* check permissions of relation */
- if (!vacuum_is_permitted_for_relation(relid, classForm, options))
- continue;
-
/*
* Build VacuumRelation(s) specifying the table OIDs to be processed.
* We omit a RangeVar since it wouldn't be appropriate to complain
@@ -2107,11 +2104,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
priv_relid = RelationGetRelid(rel);
/*
- * Check if relation needs to be skipped based on privileges. This check
- * happens also when building the relation list to vacuum for a manual
- * operation, and needs to be done additionally here as VACUUM could
- * happen across multiple transactions where privileges could have changed
- * in-between. Make sure to only generate logs for VACUUM in this case.
+ * Check if relation needs to be skipped based on privileges. Make sure
+ * to only generate logs for VACUUM in this case.
*/
if (!vacuum_is_permitted_for_relation(priv_relid,
rel->rd_rel,
--
2.54.0
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-29 16:22 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-30 23:03 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
0 siblings, 1 reply; 44+ messages in thread
From: Bharath Rupireddy @ 2026-06-29 16:22 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; suryapoondla4 <suryapoondla4@gmail.com>
Hi,
On Sun, Jun 28, 2026 at 9:58 PM cca5507 <cca5507@qq.com> wrote:
>
> Remove some outdated comments in v7.
Thanks for putting this together. The patch LGTM.
I verified it catches the vacuum_is_permitted_for_relation callers with no
lock [1]. Also, we now correctly handle concurrently dropped tables -
although we add tables that are dropped concurrently during the pg_class
scan to the vacuumable rels list, vacuum_rel and vacuum_open_relation catch
them and filter them out, instead of erroring out for database-wide vacuum.
[1]
performing post-bootstrap initialization ... ./TRAP: failed
Assert("CheckRelationOidLockedByMe(relid, AccessShareLock, true)"), File:
"vacuum.c", Line: 726, PID: 20830
/local/home/rupiredd/postgres/inst/bin/postgres(ExceptionalCondition+0x9e)[0xc71723]
/local/home/rupiredd/postgres/inst/bin/postgres(vacuum_is_permitted_for_relation+0x5f)[0x779f28]
/local/home/rupiredd/postgres/inst/bin/postgres[0x77a70b]
/local/home/rupiredd/postgres/inst/bin/postgres(vacuum+0x232)[0x779b92]
/local/home/rupiredd/postgres/inst/bin/postgres(ExecVacuum+0xc02)[0x779951]
/local/home/rupiredd/postgres/inst/bin/postgres(standard_ProcessUtility+0x853)[0xa96fc6]
/local/home/rupiredd/postgres/inst/bin/postgres(ProcessUtility+0x104)[0xa9676c]
/local/home/rupiredd/postgres/inst/bin/postgres[0xa9529f]
/local/home/rupiredd/postgres/inst/bin/postgres[0xa954fc]
/local/home/rupiredd/postgres/inst/bin/postgres(PortalRun+0x2c8)[0xa94a46]
/local/home/rupiredd/postgres/inst/bin/postgres[0xa8dd57]
/local/home/rupiredd/postgres/inst/bin/postgres(PostgresMain+0x9f4)[0xa92c6b]
/local/home/rupiredd/postgres/inst/bin/postgres(PostgresMain+0x0)[0xa92277]
/local/home/rupiredd/postgres/inst/bin/postgres(main+0x2ed)[0x82c59b]
/lib64/libc.so.6(__libc_start_main+0xea)[0x7faee3b3c13a]
/local/home/rupiredd/postgres/inst/bin/postgres(_start+0x2a)[0x49867a]
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2026-06-30 23:03 ` surya poondla <suryapoondla4@gmail.com>
2026-06-30 23:35 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-07-01 07:21 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
0 siblings, 2 replies; 44+ messages in thread
From: surya poondla @ 2026-06-30 23:03 UTC (permalink / raw)
To: nathandbossart@gmail.com <nathandbossart@gmail.com>; +Cc: cca5507 <cca5507@qq.com>; Michael Paquier <michael@paquier.xyz>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; pgsql@j-davis.com <pgsql@j-davis.com>
Hi all,
v8 is a better landing than v6 (which I'd LGTM'd). Michael's repro made it
clear that removing the early ACL filter would let
an unprivileged VACUUM actually stack behind a privileged lock, which
a556549 commit was preventing.
v8 preserves that protection while handling the concurrent-drop in
get_all_vacuum_rels, and feels like the right shape.
A few small notes on the patch:
1. The function header comment for vacuum_is_permitted_for_relation() is
unchanged and still describes only
two return paths: "issue a WARNING log message and return false". v8
introduces a third path that returns false when
is_missing fires. Worth updating the comment to describe all three return
cases and what the new missing_ok parameter controls.
2. The silent skip in get_all_vacuum_rels produces a different user-visible
behavior than vacuum_open_relation's WARNING for
what's essentially the same race (concurrent drop during a database-wide
VACUUM).
I think the silent path is fine here, as the user didn't explicitly ask for
that table.
3. The Assert that Bharath suggested earlier doesn't apply as-is but can be
incorporated into the patch as:
Assert(missing_ok || CheckRelationOidLockedByMe(relid, AccessShareLock,
true));
That would catch any future caller that passes missing_ok=false without
first acquiring a lock.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-30 23:03 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
@ 2026-06-30 23:35 ` Michael Paquier <michael@paquier.xyz>
2026-07-02 06:03 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
1 sibling, 1 reply; 44+ messages in thread
From: Michael Paquier @ 2026-06-30 23:35 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; +Cc: nathandbossart@gmail.com <nathandbossart@gmail.com>; cca5507 <cca5507@qq.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; pgsql@j-davis.com <pgsql@j-davis.com>
On Tue, Jun 30, 2026 at 04:03:14PM -0700, surya poondla wrote:
> v8 is a better landing than v6 (which I'd LGTM'd). Michael's repro made it
> clear that removing the early ACL filter would let
> an unprivileged VACUUM actually stack behind a privileged lock, which
> a556549 commit was preventing.
> v8 preserves that protection while handling the concurrent-drop in
> get_all_vacuum_rels, and feels like the right shape.
v8 is taking the right approach here.
> 3. The Assert that Bharath suggested earlier doesn't apply as-is but can be
> incorporated into the patch as:
> Assert(missing_ok || CheckRelationOidLockedByMe(relid, AccessShareLock,
> true));
This fail-safe would be a welcome addition, yes. Perhaps Nathan feels
differently, which would be also fine by me.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../akRSsP3NBe0tvKRI@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-30 23:03 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-06-30 23:35 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
@ 2026-07-02 06:03 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 44+ messages in thread
From: Kyotaro Horiguchi @ 2026-07-02 06:03 UTC (permalink / raw)
To: michael@paquier.xyz; +Cc: suryapoondla4@gmail.com; nathandbossart@gmail.com; cca5507@qq.com; pgsql-hackers@lists.postgresql.org; bharath.rupireddyforpostgres@gmail.com; pgsql@j-davis.com
Hello,
At Wed, 1 Jul 2026 08:35:12 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> > 3. The Assert that Bharath suggested earlier doesn't apply as-is but can be
> > incorporated into the patch as:
> > Assert(missing_ok || CheckRelationOidLockedByMe(relid, AccessShareLock,
> > true));
>
> This fail-safe would be a welcome addition, yes. Perhaps Nathan feels
> differently, which would be also fine by me.
To me, this Assert seems worthwhile as part of the function's contract.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-30 23:03 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
@ 2026-07-01 07:21 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-07-02 06:03 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
1 sibling, 1 reply; 44+ messages in thread
From: cca5507 @ 2026-07-01 07:21 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; nathandbossart@gmail.com <nathandbossart@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; pgsql@j-davis.com <pgsql@j-davis.com>
Hi,
> 2. The silent skip in get_all_vacuum_rels produces a different user-visible
> behavior than vacuum_open_relation's WARNING for
> what's essentially the same race (concurrent drop during a database-wide
> VACUUM).
> I think the silent path is fine here, as the user didn't explicitly ask for
> that table.
I personally think it's worth a WARNING because the relation is exist when
we scan pg_class. I'm also OK if most people think the WARNING is useless.
--
Regards,
ChangAo Chen
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-30 23:03 ` Re: Handle concurrent drop when doing whole database vacuum surya poondla <suryapoondla4@gmail.com>
2026-07-01 07:21 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-07-02 06:03 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 44+ messages in thread
From: Kyotaro Horiguchi @ 2026-07-02 06:03 UTC (permalink / raw)
To: cca5507@qq.com; +Cc: suryapoondla4@gmail.com; nathandbossart@gmail.com; michael@paquier.xyz; pgsql-hackers@lists.postgresql.org; bharath.rupireddyforpostgres@gmail.com; pgsql@j-davis.com
Hello,
At Wed, 1 Jul 2026 15:21:06 +0800, "cca5507" <cca5507@qq.com> wrote in
> > 2. The silent skip in get_all_vacuum_rels produces a different user-visible
> > behavior than vacuum_open_relation's WARNING for
> > what's essentially the same race (concurrent drop during a database-wide
> > VACUUM).
> > I think the silent path is fine here, as the user didn't explicitly ask for
> > that table.
>
> I personally think it's worth a WARNING because the relation is exist when
> we scan pg_class. I'm also OK if most people think the WARNING is useless.
I think that from the immediate caller's point of view, there is no
distinction between a relation disappearing before or after the
pg_class scan. Personally, I think vacuum_open_relation() could also
be silent in that case, although I don't see any particular need to
change its current behavior.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: Handle concurrent drop when doing whole database vacuum
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 21:06 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-26 02:31 ` Re: Handle concurrent drop when doing whole database vacuum Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 08:19 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Re: Handle concurrent drop when doing whole database vacuum Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Re: Handle concurrent drop when doing whole database vacuum Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` Re: Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
@ 2026-06-29 05:01 ` surya poondla <suryapoondla4@gmail.com>
1 sibling, 0 replies; 44+ messages in thread
From: surya poondla @ 2026-06-29 05:01 UTC (permalink / raw)
To: cca5507 <cca5507@qq.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi All,
The new patch (v6) with the Assertion check looks good to me.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 44+ messages in thread
end of thread, other threads:[~2026-08-03 21:10 UTC | newest]
Thread overview: 44+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-14 07:12 Handle concurrent drop when doing whole database vacuum =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-15 05:54 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-15 06:29 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-23 23:13 ` Michael Paquier <michael@paquier.xyz>
2026-06-23 23:33 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-23 23:40 ` Michael Paquier <michael@paquier.xyz>
2026-06-24 03:21 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-24 03:40 ` Michael Paquier <michael@paquier.xyz>
2026-06-29 16:54 ` Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 04:47 ` Michael Paquier <michael@paquier.xyz>
2026-06-30 15:30 ` Nathan Bossart <nathandbossart@gmail.com>
2026-07-09 18:49 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-07-09 18:56 ` Nathan Bossart <nathandbossart@gmail.com>
2026-08-03 21:10 ` Nathan Bossart <nathandbossart@gmail.com>
2026-06-17 18:40 ` surya poondla <suryapoondla4@gmail.com>
2026-06-18 06:44 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-22 21:26 ` surya poondla <suryapoondla4@gmail.com>
2026-06-23 05:20 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-24 22:58 ` surya poondla <suryapoondla4@gmail.com>
2026-06-23 21:06 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 05:53 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-25 06:19 ` Michael Paquier <michael@paquier.xyz>
2026-06-25 16:51 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 17:41 ` surya poondla <suryapoondla4@gmail.com>
2026-06-25 18:04 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-25 22:23 ` Michael Paquier <michael@paquier.xyz>
2026-06-26 01:39 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-26 02:31 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-26 09:36 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-26 21:19 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-27 02:09 ` Michael Paquier <michael@paquier.xyz>
2026-06-27 08:19 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-27 19:32 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 00:13 ` Michael Paquier <michael@paquier.xyz>
2026-06-29 00:28 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-29 04:37 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 04:58 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-06-29 16:22 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-06-30 23:03 ` surya poondla <suryapoondla4@gmail.com>
2026-06-30 23:35 ` Michael Paquier <michael@paquier.xyz>
2026-07-02 06:03 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-07-01 07:21 ` =?utf-8?B?Y2NhNTUwNw==?= <cca5507@qq.com>
2026-07-02 06:03 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-29 05:01 ` surya poondla <suryapoondla4@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox