| From: | surya poondla <suryapoondla4(at)gmail(dot)com> |
|---|---|
| To: | cca5507 <cca5507(at)qq(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Handle concurrent drop when doing whole database vacuum |
| Date: | 2026-06-22 21:26:45 |
| Message-ID: | CAOVWO5qFQK4txgfWPoL47owM8mP0e4M959B_hRANBzpFbaUqww@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-06-22 21:34:23 | Re: [RFC PATCH v0 0/7] Add EXPLAIN ANALYZE wait event reporting |
| Previous Message | Baji Shaik | 2026-06-22 21:21:17 | [PATCH] Warn when io_min_workers exceeds io_max_workers |