Re: BUG #14941: Vacuum crashes

Nathan Bossart <bossartn@amazon.com>

From: "Bossart, Nathan" <bossartn@amazon.com>

To: Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>

Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Lyes Ameddah <lyes.amd@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>

Date: 2018-04-05T20:29:38Z

Lists: pgsql-bugs, pgsql-hackers

Attachments

v7-0001-Add-skip_locked-argument-to-find_inheritance_chil.patch (application/octet-stream) patch v7-0001
v7-0002-Add-skip_locked-argument-to-find_all_inheritors.patch (application/octet-stream) patch v7-0002
v7-0003-Add-skip_locked-argument-to-vac_open_indexes.patch (application/octet-stream) patch v7-0003
v7-0004-Rename-VACOPT_NOWAIT-to-VACOPT_SKIP_LOCKED.patch (application/octet-stream) patch v7-0004
v7-0005-Add-assertion-that-we-are-not-an-autovacuum-worke.patch (application/octet-stream) patch v7-0005
v7-0006-Create-a-helper-function-for-determining-the-log-.patch (application/octet-stream) patch v7-0006
v7-0007-Create-a-helper-function-for-cleaning-up-from-do_.patch (application/octet-stream) patch v7-0007
v7-0008-Skip-VACUUM-ANALYZE-with-VACOPT_SKIP_LOCKED-if-in.patch (application/octet-stream) patch v7-0008
v7-0009-Skip-ANALYZE-with-VACOPT_SKIP_LOCKED-if-acquire_i.patch (application/octet-stream) patch v7-0009
v7-0010-Skip-VACUUM-ANALYZE-with-VACOPT_SKIP_LOCKED-if-ex.patch (application/octet-stream) patch v7-0010
v7-0011-Skip-VACUUM-FULL-with-VACOPT_SKIP_LOCKED-if-clust.patch (application/octet-stream) patch v7-0011
v7-0012-Open-up-VACOPT_SKIP_LOCKED-to-users.patch (application/octet-stream) patch v7-0012

On 3/30/18, 7:19 PM, "Andres Freund" <andres@anarazel.de> wrote:
> Could you post a rebased version of the patch, with an incremental
> addition of what you propose in a separate patch?

Here is a new patch set.  I have attempted to add skip-locked
functionality everywhere it is needed, including VACUUM FULL.

On 4/3/18, 12:12 AM, "Michael Paquier" <michael@paquier.xyz> wrote:
> On Fri, Mar 30, 2018 at 05:19:46PM -0700, Andres Freund wrote:
>> On 2018-03-30 18:39:01 +0000, Bossart, Nathan wrote:
>>> I think the most straightforward approach for fixing this is to add
>>> skip-locked functionality in find_all_inheritors(),
>>> find_inheritance_children(), and vac_open_indexes(), but I am curious
>>> what others think.
>
> For vac_open_indexes() it may make the most sense to introduce a new
> try_open_index which wraps on top of try_relation_open with checks on
> the opened relation's relkind. 

I looked into this, but I noticed that the analogous
try_relation_open() implements missing-ok for relation_open().  So, I
thought it might be confusing to have try_index_open() be a skip-
locked version of index_open().

The new tests are now passing as expected, but I am still doing some
manual testing.  Since there is quite a bit of added complexity in
this new patch set, I will certainly not be surprised if this gets
moved to the next commitfest.

Nathan

Commits

Add option SKIP_LOCKED to VACUUM and ANALYZE
- 803b1301e8c9 12.0 landed
Rename VACOPT_NOWAIT to VACOPT_SKIP_LOCKED
- edc6b41bd4a8 12.0 landed
Add parenthesized options syntax for ANALYZE.
- 854dd8cff523 11.0 landed
Don't allow VACUUM VERBOSE ANALYZE VERBOSE.
- 921059bd66c7 11.0 landed
Fix inadequate locking during get_rel_oids().
- 19de0ab23ccb 11.0 cited
Avoid having autovacuum workers wait for relation locks.
- 32896c40ca76 9.1.0 cited