Re: BUG #14941: Vacuum crashes
Michael Paquier <michael.paquier@gmail.com>
From: Michael Paquier <michael.paquier@gmail.com>
To: "Bossart, Nathan" <bossartn@amazon.com>
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-01-10T02:53:31Z
Lists: pgsql-bugs, pgsql-hackers
On Tue, Jan 09, 2018 at 09:40:50PM +0000, Bossart, Nathan wrote: > On 1/8/18, 10:28 PM, "Michael Paquier" <michael.paquier@gmail.com> wrote: >> I think that you are doing it wrong here. In get_all_vacuum_rels() you >> should build a RangeVar to be reused in the context of this error >> message, and hence you'll save an extra lookup based on the relation >> OID here, saving from any timing issues that you have overseen as in >> this code path a lock on the relation whose name is looked at is not >> taken. Relying on the RangeVar being NULL to not generate any logs is >> fine as a concept to me, but let's fill it where it is needed, and in >> the case of this patch a VACUUM NOWAIT on the whole database is such a >> case. > > I understand what you are saying here. I think there are two competing > logging behaviors: > > 1. If a relation is concurrently dropped, we should skip logging if > the relation was not specified in the original VACUUM/ANALYZE > command [0] > 2. If a relation is skipped because the lock is not available, we > should never skip logging At the end this comes back to if the relation is explicitely listed or not in the command specified by the user.. > What do you think? I will take a deeper look into how your suggested > approach might be achieved. Backpedalling a bit on this point and coming back to this message from Tom (https://www.postgresql.org/message-id/28748.1507071576%40sss.pgh.pa.us) which you just cited. Why do we actually need to issue any WARNING messages for unlisted relations? Contrary to what Sawada-san complained upthread, it looks sane to me to not log anything if a relation is not explicitely listed. So you should not get any warnings for a database-wide VACUUM if a relation is dropped while the thing is running, and what you proposed initially in https://www.postgresql.org/message-id/D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com is more simple, does not need to worry about any kind of timing issues, and is consistent with autovacuum. -- Michael
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