On Tue, Jul 16, 2013 at 05:56:10PM +0200, Andres Freund wrote:
> On 2013-07-16 09:50:07 -0400, Noah Misch wrote:
> > With "\c", in general, you may end up executing commands under the new session
> > before the old backend has finished exiting. For this test case specifically,
> > the two backends' attempts to drop table "t" regularly overlap. The old
> > backend would drop it within RemoveTempRelationsCallback(), and the new
> > backend would cascade from "rowtype" to drop it. findDependentObjects() deals
> > with concurrent deletion attempts by acquiring a lock on each object it will
> > delete, then calling systable_recheck_tuple() to determine whether another
> > deleter was successful while the current backend was waiting for the lock.
> > systable_recheck_tuple() uses the scan snapshot, which really only works if
> > that snapshot is SnapshotNow or some other that changes its decision in
> > response to concurrent transaction commits. The switch to MVCC snapshots left
> > this mutual exclusion protocol ineffective.
>
> Nice catch.
>
> I wonder though, isn't that code unsafe in other ways as well? What if
> the pg_depend entry was rewritten inbetween? Consider somebody doing
> something like REASSIGN OWNED concurrently with a DROP. The DROP
> possibly will cascade to an entry which changed the owner already. And
> the recheck will then report that the object doesn't exist anymore and
> abort since it does a simple HeapTupleSatisfiesVisibility() and doesn't
> follow the ctid chain if the tuple has been updated...
I'm not seeing a problem with that particular route. Say we're examining a
pg_depend tuple where the referencing object is a table and the referenced
object is a role, the table's owner. We're dropping the role and cascade to
the table. If the REASSIGNED OWNED assigns the table to a different role,
then we are correct to treat the dependency as gone. If it's the same role
("REASSIGNED OWNED BY alice TO alice", pointless but permitted), several of
the rename implementations short-circuit that case and don't change catalog
entries. But even if that optimization were omitted, shdepChangeDep() will
have blocked against our previously-acquired deletion lock on the role. Code
that adds or updates a dependency without locking both objects of the new
pg_depend tuple is buggy independently.
That being said, there may well be a related mechanism that can slip past the
locking here. My brain turns to mush when I ponder findDependentObjects() too
thoroughly.
Thanks,
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com