On Thu, Nov 20, 2014 at 09:15:51PM +0100, Andres Freund wrote: > On 2014-11-20 13:57:25 -0500, Robert Haas wrote: > > On Thu, Nov 20, 2014 at 12:19 PM, Andres Freund <and...@2ndquadrant.com> > > wrote: > > If the > > deadlock detector would kill the processes anyway, it doesn't matter, > > because CheckTableNotInUse() should do it first, so that we get a > > better error and without waiting for deadlock_timeout. So any > > scenario that's predicated on the assumption that CheckTableNotInUse() > > will succeed in a parallel context is 100% unconvincing to me as an > > argument for anything. > > Meh, there's plenty cases where CheckTableNotInUse() isn't used where we > still rely on locks actually working. And it's not just postgres code, > this *will* break user code.
Today, holding a relation AccessExclusiveLock ensures no other process is using the table. That is one of two prerequisites for making an alteration that threatens concurrent users of the table. The other prerequisite is verifying that no other subsystem of the current process is using the table, hence CheckTableNotInUse(). Robert's designs do soften AccessExclusiveLock for relations; holding it will not preclude use of the table in workers of the same parallel group. Pessimizing CheckTableNotInUse() compensates. In every scenario where AccessExclusiveLock is softer than before, CheckTableNotInUse() will fail unconditionally. Code that, today, performs the correct checks before making a threatening table alteration will remain correct. ANALYZE and lazy VACUUM are the only core operations that take self-exclusive relation locks and skip CheckTableNotInUse(). PreventTransactionChain() will block naive concurrent VACUUM. (If that weren't the case, VACUUM would need CheckTableNotInUse() even without parallelism in the picture. I wouldn't oppose adding CheckTableNotInUse() to VACUUM as defense in depth.) ANALYZE is special. It's compatible with most concurrent use of the table, so CheckTableNotInUse() is unnecessary. Blocking ANALYZE during parallelism, even if we someday allow other database writes, is a wart I can live with. Given a choice between no parallelism and parallelism when the initiating transaction holds no self-exclusive locks, I'd pick the latter. But you have overstated the significance of holding a lock and the degree to which Robert's proposals change the same. > Your argument essentially is that we don't actually need to lock objects > if the other side only reads. Yes, you add a condition or two ontop > (e.g. CheckTableNotInUse() fails), but that's not that much. If we want > to do this we really need to forbid *any* modification to catalog/user > tables while parallel operations are ongoing. In the primary and worker > backends. I think that's going to end up being much more > problematic/restrictive than not allowing non-cooperative paralellism > after >= ShareUpdateExclusive. If we ever want to parallelize writing > operations we'll regret this quite a bit. https://wiki.postgresql.org/wiki/Parallel_Sort already contemplates blocking every kind of database write, due to challenges around combo CIDs (affects UPDATE and DELETE) and invalidation messages (mostly affects DDL). I doubt we'll ever care to allow a worker to start an independent DDL operation. We might want to allow INSERT, UPDATE, and DELETE. I don't see that allowing multiple workers to hold AccessExclusiveLock will make that more difficult. Difficulties around XactLockTableWait() are more noteworthy, and they stand regardless of what we do with heavyweight locking. > The 'lets grant all the current locks at start of parallel query' > approach seems quite a bit safer than always doing that during parallel > query, don't get me wrong. Agreed. > Btw, what are your thoughts about SERIALIZABLE and parallel query? > Afaics that'd be pretty hard to support? It didn't look obviously awful, because most of the relevant data structures live in shared memory. We already have the concept of one backend adjusting the predicate locks pertaining to another backend's transaction. (That's not to say the number of SERIALIZABLE-specific changes will be zero, either.) Thanks, nm -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers