Re: DROP OWNED CASCADE vs Temp tables

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Mithun Cy <mithun(dot)cy(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: DROP OWNED CASCADE vs Temp tables
Date: 2020-05-06 17:02:26
Message-ID: 20200506170226.GA15066@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On 2020-Apr-06, Tom Lane wrote:

> Alvaro Herrera <alvherre(at)2ndquadrant(dot)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

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2020-05-06 21:26:55 BUG #16419: wrong parsing BC year in to_date() function
Previous Message David G. Johnston 2020-05-06 14:05:45 Re: Clarity Bug for Schema Permissions, Potential Vulnerability?

Browse pgsql-hackers by date

  From Date Subject
Next Message Victor Wagner 2020-05-06 17:14:22 Re: Postgres Windows build system doesn't work with python installed in Program Files
Previous Message Robert Haas 2020-05-06 15:15:57 Re: tablespace_map code cleanup