Patch to avoid orphaned dependencies

From: "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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: Patch to avoid orphaned dependencies
Date: 2021-05-04 09:55:43
Message-ID: 8369ff70-0e31-f194-2954-787f4d9e21dd@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

This new thread is a follow-up of [1].

Problem description:

We have occasionally observed objects having an orphaned dependency, the
most common case we have seen (if not the only one) is functions not
linked to any namespaces.
A patch has been initially proposed to fix this particular
(function-to-namespace) dependency (see [1]), but there could be much
more scenarios (like the function-to-datatype one highlighted by Gilles
in [1] that could lead to a function having an invalid parameter datatype).
As Tom said there are dozens more cases that would need to be
considered, and a global approach to avoid those race conditions should
be considered instead.

The attached patch is avoiding those race conditions globally by
changing the dependency mechanism: we are using a dirty snapshot any
time we’re about to create a pg_depend or pg_shdepend entry.
That way we can check if there is in-flight transactions that are
affecting the dependency: if that’s the case, an error is being reported.

This approach has been chosen over another one that would have make use
of the locking machinery.
The reason for this choice is to avoid possible slow down of typical DDL
command, risk of deadlock, number of locks taken by transaction...

Implementation overview:

* A new catalog snapshot is added: DirtyCatalogSnapshot.
* This catalog snapshot is a dirty one to be able to look for
in-flight dependencies.
* Its usage is controlled by a new UseDirtyCatalogSnapshot variable.
* Any time this variable is being set to true, then the next call to
GetNonHistoricCatalogSnapshot() is returning the dirty snapshot.
* This snapshot is being used to check for in-flight dependencies and
also to get the objects description to generate the error messages.

*Testing:*

Test 1

Session1:

|postgres=# create schema tobeorph; CREATE SCHEMA postgres=# create
table tobeorph.bdt (a int); CREATE TABLE postgres=# begin; BEGIN
postgres=*# CREATE OR REPLACE FUNCTION tobeorph.bdttime() RETURNS
TIMESTAMP AS $$DECLARE outTS TIMESTAMP; BEGIN perform pg_sleep(10);
RETURN CURRENT_TIMESTAMP; END; $$ LANGUAGE plpgsql volatile; CREATE
FUNCTION |

Session 1 does not commit, then session 2:

|postgres=# drop schema tobeorph; ERROR: cannot drop schema tobeorph
because other objects depend on it DETAIL: table tobeorph.bdt depends on
schema tobeorph function tobeorph.bdttime() (not yet committed) depends
on schema tobeorph HINT: DROP and DROP CASCADE won't work when there are
uncommitted dependencies. |

Test 2

Session 1:

|postgres=# create schema toinsert; CREATE SCHEMA postgres=# begin;
BEGIN postgres=*# drop schema toinsert; DROP SCHEMA |

Session 1 does not commit, then session 2:

|postgres=# CREATE OR REPLACE FUNCTION toinsert.bdttime() RETURNS
TIMESTAMP AS $$DECLARE outTS TIMESTAMP; BEGIN perform pg_sleep(10);
RETURN CURRENT_TIMESTAMP; END; $$ LANGUAGE plpgsql volatile; ERROR:
cannot create function toinsert.bdttime() because it depends of other
objects uncommitted dependencies DETAIL: function toinsert.bdttime()
depends on schema toinsert (dependency not yet committed) HINT: CREATE
won't work as long as there is uncommitted dependencies. |

Test3

|Session1: psql -U toorph postgres psql (14devel) Type "help" for help.
postgres=> begin; BEGIN postgres=*> CREATE OR REPLACE FUNCTION bdttime()
RETURNS TIMESTAMP AS $$DECLARE outTS TIMESTAMP; BEGIN perform
pg_sleep(10); RETURN CURRENT_TIMESTAMP; END; $$ LANGUAGE plpgsql
volatile; CREATE FUNCTION |

Session 1 does not commit, then session 2:

|postgres=# drop owned by toorph; ERROR: cannot drop objects owned by
role toorph because other uncommitted objects depend on it DETAIL:
function public.bdttime() (not yet committed) depends on role toorph
HINT: Commit or rollback function public.bdttime() creation. |

I'm creating a new commitfest entry for this patch.

Thanks

Bertrand

||

[1]:
https://www.postgresql.org/message-id/flat/a4f55089-7cbd-fe5d-a9bb-19adc6418ae9%40darold.net#9af5cdaa9e80879beb1def3604c976e8

Attachment Content-Type Size
v1-0001-orphaned-dependencies.patch text/plain 27.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-05-04 10:27:03 Re: Dumping/restoring fails on inherited generated column
Previous Message Bharath Rupireddy 2021-05-04 09:07:11 Re: Identify missing publications from publisher while create/alter subscription.