Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>, Jeremy Schneider <schnjere@amazon.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, "Albin, Lloyd P" <lalbin@scharp.org>
Date: 2018-07-26T05:24:16Z
Lists: pgsql-bugs, pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Improve VACUUM and ANALYZE by avoiding early lock queue
- a556549d7e6d 12.0 landed
-
Improve TRUNCATE by avoiding early lock queue
- f841ceb26d70 12.0 landed
-
Restrict access to reindex of shared catalogs for non-privileged users
- 87330e21c327 11.0 landed
- 661dd23950f2 12.0 landed
-
Improve behavior of concurrent CLUSTER.
- cbe24a6dd8fb 9.2.0 cited
Attachments
- 0001-Refactor-TRUNCATE-execution-to-avoid-early-lock-look.patch (text/x-diff) patch 0001
- 0002-Refactor-VACUUM-execution-to-avoid-early-lock-lookup.patch (text/x-diff) patch 0002
- 0003-Restrict-access-to-system-wide-REINDEX-for-non-privi.patch (text/x-diff) patch 0003
Hi, So, I have spent the last couple of days trying to figure out a nice solution for VACUUM, TRUNCATE and REINDEX, and attached is a set of three patches to address the issues of this thread: 1) 0001 does the work for TRUNCATE, but using RangeVarGetRelidExtended with a custom callback based on the existing truncate_check_rel(). I had to split the session-level checks into a separate routine as no Relation is available, but that finishes by being a neat result without changing any user-facing behavior. 2) 0002 does the work for VACUUM. It happens that vacuum_rel() already does all the skip-related checks we need to know about to decide if a relation needs to be vacuum'ed or not, so I refactored things as follows: 2-1) For a VACUUM manual listing relations, issue an ERROR if it cannot be vacuum'ed. Previously vacuum_rel() would just log a WARNING and call it a day *after* locking the relation. But as we need to rely on RangeVarGetRelidExtended() an ERROR is necessary. The ERROR happens only if VACUUM FULL is used. 2-2) When a relation list is not specified in a manual VACUUM command, then the decision to skip the relation is done in get_all_vacuum_rels() when building the relation list with the pg_class lookup. This logs a DEBUG message when the relation is skipped, which is more information that what we have now. The checks need to happen again in vacuum_rel as the VACUUM work could be spawned across multiple transactions, where a WARNING is logged. 3) REINDEX is already smart enough to check for ownership of relations if one is manually listed and reports an ERROR. However it can cause the instance to be stuck when doing a database-wide REINDEX on a database using just the owner of this database. In this case it seems to me that we need to make ReindexMultipleTables in terms of ACL checks, as per 0003. I quite like the shape of the patches proposed here, and the refactoring is I think pretty clear. Each patch can be treated independently as well. Comments are welcome. (Those patches are not indented yet, which does not matter much at this stage anyway.) Thanks, -- Michael