Improve behavior of concurrent ANALYZE/VACUUM
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@postgresql.org>
Cc: bossartn@amazon.com, horiguchi.kyotaro@lab.ntt.co.jp
Date: 2018-08-12T22:21:42Z
Lists: pgsql-hackers
Attachments
- 0001-Improve-VACUUM-and-ANALYZE-by-avoiding-early-lock-qu.patch (text/x-diff) patch 0001
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
Commits
-
Improve VACUUM and ANALYZE by avoiding early lock queue
- a556549d7e6d 12.0 landed
-
Add more tests for VACUUM skips with partitioned tables
- a569eea6998a 12.0 landed
-
Add regression tests for VACUUM and ANALYZE with relation skips
- 98abc738027e 12.0 landed