From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Postgres hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | bossartn(at)amazon(dot)com, horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp |
Subject: | Improve behavior of concurrent ANALYZE/VACUUM |
Date: | 2018-08-12 22:21:42 |
Message-ID: | 20180812222142.GA6097@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi all,
(In CC are the folks who have reviewed the first patch versions, Nathan
and Horiguchi-san.)
After TRUNCATE and REINDEX, here is the third and last thread I am
spawning for the previous thread "Canceling authentication due to
timeout aka Denial of Service Attack":
https://www.postgresql.org/message-id/152512087100.19803.12733865831237526317%40wrigleys.postgresql.org
And this time the discussion is about VACUUM/ANALYZE. In this case, we
also check relation ownership after queuing for a lock, which can allow
any user to potentially lock a relation which others could use,
particularly with VACUUM FULL which needs an AEL (access exclusive
lock).
In the previous thread, we discussed a couple of approaches, but I was
not happy with any of those, hence I have been spending more time in
getting to a solution which has no user-facing changes, and still solves
the problems folks have been complaining about, and the result is the
patch attached. The patch changes a couple of things regarding ACL
checks, by simply centralizing the ownership checks into a single
routine used by both ANALYZE and VACUUM. This routine is then used in
two more places for manual ANALYZE and VACUUM:
- When specifying directly one or more relations in the command, in
expand_vacuum_rel().
- When building the complete list of relations to work on in the case of
a database-wide operation, in get_all_vacuum_rels().
analyze_rel() and vacuum_rel() have been using the same logic to check
for relation ownership, so refactoring things into a single routine is a
win in my opinion.
While reviewing the code, I have of course noticed that analyze_rel()
makes an effort to not produce a WARNING if both VACOPT_VACUUM and
VACOPT_ANALYZE are specified in VacuumStmt->options, however we can
never see that scenario as analyze_rel() never gets called at the same
time as vacuum_rel() for a single relation.
The patch attached includes tests which I have used to also check that
correct error messages are produced for VACUUM, VACUUM ANALYZE and
ANALYZE.
Please note that like the previous one for TRUNCATE, I would no plans
for a back-patch with the same arguments as previously. There are also
serious bugs being worked on for REL_11_STABLE so I don't want to take
any risk for this branch.
Thoughts?
--
Michael
Attachment | Content-Type | Size |
---|---|---|
0001-Improve-VACUUM-and-ANALYZE-by-avoiding-early-lock-qu.patch | text/x-diff | 21.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2018-08-12 22:53:00 | Re: [HACKERS] Optional message to user when terminating/cancelling backend |
Previous Message | Chapman Flack | 2018-08-12 20:26:31 | Re: lazy detoasting |