On 2020-Apr-06, Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2020-Jan-14, Alvaro Herrera wrote:
> >> Hmm, it seems to be doing it differently. Maybe it should be acquiring
> >> locks on all objects in that nested loop and verified them for
> >> existence, so that when it calls performMultipleDeletions the objects
> >> are already locked, as you say.
>
> > Yeah, this solves the reported bug.
>
> I looked this over and think it should be fine. There will be cases
> where we get a deadlock error, but such risks existed anyway, since
> we'd have acquired all the same locks later in the process.
Thanks for looking again. I have pushed this to all branches, with
these changes:
> Hmmm ... there is an argument for doing ReleaseDeletionLock in the code
> paths where you discover that the object's been deleted.
Added this. This of course required also exporting ReleaseDeletionLock,
which closes my concern about exporting only half of that API.
> Also, if we're exporting these, it's worth expending a bit more
> effort on their header comments. In particular AcquireDeletionLock
> should describe its flags argument; perhaps along the lines of
> "Accepts the same flags as performDeletion (though currently only
> PERFORM_DELETION_CONCURRENTLY does anything)".
Did this too. I also changed the comment to indicate that, since
they're now exported APIs, they might grow the ability to lock shared
objects in the future. In fact, we have some places where we're using
LockSharedObject directly to lock objects to drop; it seems reasonable
to think that we should augment AcquireDeletionLock to handle those
objects and make those places use the new API.
Lastly: right now, only performMultipleDeletions passes the flags down
to AcquireDeletionLock -- there are a couple places that drop objects
and call AcquireDeletionLock with flags=0. There's no bug AFAICS
because those cannot be called while running concurrent object drop.
But for correctness, those should pass flags too.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
