Re: Patch to avoid orphaned dependencies

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Gilles Darold <gilles(at)darold(dot)net>, "Nasby, Jim" <nasbyj(at)amazon(dot)com>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Subject: Re: Patch to avoid orphaned dependencies
Date: 2022-03-23 16:49:06
Message-ID: 2765982.1648054146@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Nathan Bossart <nathandbossart(at)gmail(dot)com> writes:
> Bertand, do you think this has any chance of making it into v15? If not,
> are you alright with adjusting the commitfest entry to v16 and moving it to
> the next commitfest?

I looked this over briefly, and IMO it should have no chance of being
committed in anything like this form.

The lesser problem is that (as already noted) the reliance on a global
variable that changes catalog lookup semantics is just unbelievably
scary. An example of the possible consequences here is that a syscache
entry could get made while that's set, containing a row that we should
not be able to see yet, and indeed might never get committed at all.
Perhaps that could be addressed by abandoning the patch's ambition to tell
you the details of an uncommitted object (which would have race conditions
anyway), so that *only* reads of pg_[sh]depend itself need be dirty.

The bigger problem is that it fails to close the race condition that
it's intending to solve. This patch will catch a race like this:

Session doing DROP Session doing CREATE

DROP begins

CREATE makes a dependent catalog entry

DROP scans for dependent objects

CREATE commits

DROP removes catalog entry

DROP commits

However, it will not catch this slightly different timing:

Session doing DROP Session doing CREATE

DROP begins

DROP scans for dependent objects

CREATE makes a dependent catalog entry

CREATE commits

DROP removes catalog entry

DROP commits

So I don't see that we've moved the goalposts very far at all.

Realistically, if we want to prevent this type of problem, then all
creation DDL will have to take a lock on each referenced object that'd
conflict with a lock taken by DROP. This might not be out of reach:
I think we do already take such locks while dropping objects. The
reference-side lock could be taken by the recordDependency mechanism
itself, ensuring that we don't miss anything; and that would also
allow us to not bother taking such a lock on pinned objects, which'd
greatly cut the cost (though not to zero).

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhihong Yu 2022-03-23 16:59:01 Re: Skip partition tuple routing with constant partition key
Previous Message Zhihong Yu 2022-03-23 16:42:21 Re: freeing bms explicitly