DROP FUNCTION failure: cache lookup failed for relation X

Started by Michael Fuhralmost 19 years ago10 messages
#1Michael Fuhr
mike@fuhr.org

I've found a situation that causes DROP FUNCTION to fail (tested
in 8.1.6, 8.2.1, and 8.3devel):

CREATE TABLE foo (id integer);

CREATE FUNCTION foofunc() RETURNS trigger AS $$
BEGIN
RETURN NEW;
END;
$$ LANGUAGE plpgsql;

Then in concurrent sessions:

A: BEGIN;

A: CREATE TRIGGER footrig BEFORE INSERT ON foo
FOR EACH ROW EXECUTE PROCEDURE foofunc();

B: DROP TABLE foo; -- blocks pending A's commit

A: COMMIT; -- B's DROP TABLE completes

A: SELECT tgrelid FROM pg_trigger WHERE tgname = 'footrig';
tgrelid
---------
66153
(1 row)

A: DROP FUNCTION foofunc();
ERROR: cache lookup failed for relation 66153

Apparently the row in pg_trigger that A committed wasn't deleted
by B's DROP TABLE, presumably because B didn't have visibility to
to the trigger when its DROP TABLE statement began. This case is
admittedly contrived but I did stumble across it in a test environment.

--
Michael Fuhr

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Fuhr (#1)
Re: DROP FUNCTION failure: cache lookup failed for relation X

Michael Fuhr <mike@fuhr.org> writes:

I've found a situation that causes DROP FUNCTION to fail (tested
in 8.1.6, 8.2.1, and 8.3devel):

Ugh ... I haven't traced this through in detail, but I'm pretty sure
the problem arises from the fact that dependency.c traces through
auto/internal dependencies before actually starting to do the deletions
(and thus before acquiring locks). Can we fix this without multiple
scans of the dependency tree (probably costing O(N^lots))?

regards, tom lane

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: DROP FUNCTION failure: cache lookup failed for

Is this a TODO?

---------------------------------------------------------------------------

Tom Lane wrote:

Michael Fuhr <mike@fuhr.org> writes:

I've found a situation that causes DROP FUNCTION to fail (tested
in 8.1.6, 8.2.1, and 8.3devel):

Ugh ... I haven't traced this through in detail, but I'm pretty sure
the problem arises from the fact that dependency.c traces through
auto/internal dependencies before actually starting to do the deletions
(and thus before acquiring locks). Can we fix this without multiple
scans of the dependency tree (probably costing O(N^lots))?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: DROP FUNCTION failure: cache lookup failed for relation X

I wrote:

Michael Fuhr <mike@fuhr.org> writes:

I've found a situation that causes DROP FUNCTION to fail (tested
in 8.1.6, 8.2.1, and 8.3devel):
http://archives.postgresql.org/pgsql-hackers/2007-01/msg00937.php

Ugh ... I haven't traced this through in detail, but I'm pretty sure
the problem arises from the fact that dependency.c traces through
auto/internal dependencies before actually starting to do the deletions
(and thus before acquiring locks).

I looked into this a bit more, and found that it's actually a pretty
general issue with the dependency.c code: we delete objects depending
on a target object before we delete the target itself. Which is fine,
except that we don't try to take out any lock on the target object until
we get to the object-type-specific subroutine that's supposed to delete
it.

I think we could fix this for tables by acquiring lock on a table at the
instant it's put into a list for deletion inside dependency.c. That
would be enough to fix Michael's problem instance, but what of other
types of objects? There doesn't seem to be anything preventing somebody
from, say, deleting a function at the same time someone else is creating
an operator depending on the function. We mostly don't take locks on
non-table objects while working with them, and for the most part this is
fairly sane because those objects are defined by a single system catalog
row anyway: either you see the row or you don't. But this means that
the depended-on object could be gone by the time you finish adding a
dependency on it.

It seems a general solution would involve having dependency.c take
exclusive locks on all types of objects (not only tables) as it scans
them and decides they need to be deleted later. And when adding a
pg_depend entry, we'd need to take a shared lock and then recheck to
make sure the object still exists. This would be localized in
dependency.c, but it still seems like quite a lot of mechanism and
cycles added to every DDL operation. And I'm not at all sure that
we'd not be opening ourselves up to deadlock problems.

I'm a bit tempted to fix only the table case and leave the handling of
non-table objects as is. Comments?

regards, tom lane

#5Richard Troy
rtroy@ScienceTools.com
In reply to: Tom Lane (#4)
Re: DROP FUNCTION failure: cache lookup failed for relation

It seems a general solution would involve having dependency.c take
exclusive locks on all types of objects (not only tables) as it scans
them and decides they need to be deleted later. And when adding a
pg_depend entry, we'd need to take a shared lock and then recheck to
make sure the object still exists. This would be localized in
dependency.c, but it still seems like quite a lot of mechanism and
cycles added to every DDL operation. And I'm not at all sure that
we'd not be opening ourselves up to deadlock problems.

I'm a bit tempted to fix only the table case and leave the handling of
non-table objects as is. Comments?

regards, tom lane

The taking of DDL locks is very unlikely to create a performance problem
for anyone as DML statements typically far outnumber DDL statements.
Further, in my experience, DDL statements are very carefully thought
through and are usually either completely automated by well crafted
programs or are performed by one person at a time - the DBA. I therefore
conclude that any deadlock risk is triflingly small and would be a
self-inflicted circumstance.

Richard

--
Richard Troy, Chief Scientist
Science Tools Corporation
510-924-1363 or 202-747-1263
rtroy@ScienceTools.com, http://ScienceTools.com/

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
Re: DROP FUNCTION failure: cache lookup failed for relation X

Uh, where are we on this?

---------------------------------------------------------------------------

Tom Lane wrote:

I wrote:

Michael Fuhr <mike@fuhr.org> writes:

I've found a situation that causes DROP FUNCTION to fail (tested
in 8.1.6, 8.2.1, and 8.3devel):
http://archives.postgresql.org/pgsql-hackers/2007-01/msg00937.php

Ugh ... I haven't traced this through in detail, but I'm pretty sure
the problem arises from the fact that dependency.c traces through
auto/internal dependencies before actually starting to do the deletions
(and thus before acquiring locks).

I looked into this a bit more, and found that it's actually a pretty
general issue with the dependency.c code: we delete objects depending
on a target object before we delete the target itself. Which is fine,
except that we don't try to take out any lock on the target object until
we get to the object-type-specific subroutine that's supposed to delete
it.

I think we could fix this for tables by acquiring lock on a table at the
instant it's put into a list for deletion inside dependency.c. That
would be enough to fix Michael's problem instance, but what of other
types of objects? There doesn't seem to be anything preventing somebody
from, say, deleting a function at the same time someone else is creating
an operator depending on the function. We mostly don't take locks on
non-table objects while working with them, and for the most part this is
fairly sane because those objects are defined by a single system catalog
row anyway: either you see the row or you don't. But this means that
the depended-on object could be gone by the time you finish adding a
dependency on it.

It seems a general solution would involve having dependency.c take
exclusive locks on all types of objects (not only tables) as it scans
them and decides they need to be deleted later. And when adding a
pg_depend entry, we'd need to take a shared lock and then recheck to
make sure the object still exists. This would be localized in
dependency.c, but it still seems like quite a lot of mechanism and
cycles added to every DDL operation. And I'm not at all sure that
we'd not be opening ourselves up to deadlock problems.

I'm a bit tempted to fix only the table case and leave the handling of
non-table objects as is. Comments?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: DROP FUNCTION failure: cache lookup failed for relation X

Bruce Momjian <bruce@momjian.us> writes:

Uh, where are we on this?

Still in the think-about-it mode, personally ... my proposed fix is
certainly much too invasive to consider back-patching, so unless someone
comes up with a way-simpler idea, it's 8.3 material at best ...

regards, tom lane

#8Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#7)
Re: DROP FUNCTION failure: cache lookup failed for relation X

Added to TODO:

* Increase locking when DROPing objects so dependent objects cannot
get dropped while the DROP operation is happening

http://archives.postgresql.org/pgsql-hackers/2007-01/msg00937.php

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Uh, where are we on this?

Still in the think-about-it mode, personally ... my proposed fix is
certainly much too invasive to consider back-patching, so unless someone
comes up with a way-simpler idea, it's 8.3 material at best ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#9Mark Kirkwood
markir@paradise.net.nz
In reply to: Tom Lane (#7)
1 attachment(s)
Cache lookup failed for relation X [was: DROP FUNCTION cache lookup failed for relation X]

Tom Lane wrote:

Still in the think-about-it mode, personally ... my proposed fix is
certainly much too invasive to consider back-patching, so unless someone
comes up with a way-simpler idea, it's 8.3 material at best ...

I ran into a variant of this today - simply creating and dropping a
table repeatedly while doing \d from another session:

Session 1:

perl -e 'while (1) {print "drop table if exists z0; \n create table z0
(a int, b int);\n drop table z0;\n"}' | psql cache > z0.log 2>&1

Session 2:

psql cache
=# \d
ERROR: cache lookup failed for relation 945897 (in RelationIsVisible,
namespace.c:406)

The previous discussion centered around working on on locking in
dependency.c whilst dropping related objects - but does this apply when
there is just one? Anyway I tried to understand what was happening and
the attached rather hacky patch seems to cure the behaviour - So I've
submitted it as a discussion aid, rather than 'the way of fixing
this'... since I'm hoping there is a better way :-)

regards

Mark

Attachments:

namspace.c.patchtext/x-patch; name=namspace.c.patchDownload
Index: src/backend/catalog/namespace.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/namespace.c,v
retrieving revision 1.99
diff -c -r1.99 namespace.c
*** src/backend/catalog/namespace.c	27 Aug 2007 03:36:08 -0000	1.99
--- src/backend/catalog/namespace.c	1 Nov 2007 07:55:34 -0000
***************
*** 19,26 ****
--- 19,28 ----
   */
  #include "postgres.h"
  
+ #include "access/heapam.h"
  #include "access/xact.h"
  #include "catalog/dependency.h"
+ #include "catalog/indexing.h"
  #include "catalog/namespace.h"
  #include "catalog/pg_authid.h"
  #include "catalog/pg_conversion.h"
***************
*** 41,46 ****
--- 43,49 ----
  #include "storage/ipc.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/fmgroids.h"
  #include "utils/guc.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
***************
*** 398,409 ****
  	Form_pg_class relform;
  	Oid			relnamespace;
  	bool		visible;
  
  	reltup = SearchSysCache(RELOID,
  							ObjectIdGetDatum(relid),
  							0, 0, 0);
  	if (!HeapTupleIsValid(reltup))
! 		elog(ERROR, "cache lookup failed for relation %u", relid);
  	relform = (Form_pg_class) GETSTRUCT(reltup);
  
  	recomputeNamespacePath();
--- 401,441 ----
  	Form_pg_class relform;
  	Oid			relnamespace;
  	bool		visible;
+ 	bool		fromcache = true;
  
  	reltup = SearchSysCache(RELOID,
  							ObjectIdGetDatum(relid),
  							0, 0, 0);
  	if (!HeapTupleIsValid(reltup))
! 	{
! 		/* See if we can get it directly. */
! 		Relation		relation;
! 		HeapScanDesc	scan;
! 		ScanKeyData 	scanKeyData;
! 
! 		fromcache = false;
! 
! 		ScanKeyInit(&scanKeyData,
! 					ObjectIdAttributeNumber,
! 					BTEqualStrategyNumber, F_OIDEQ,
!                 	ObjectIdGetDatum(ClassOidIndexId));
! 
! 		relation = heap_open(RelationRelationId, AccessShareLock);
! 
! 		scan = heap_beginscan(relation, ActiveSnapshot,
! 							  1, &scanKeyData);
! 
! 		reltup = heap_getnext(scan, ForwardScanDirection);
! 		reltup = heap_copytuple(reltup);
! 		if (!HeapTupleIsValid(reltup))
! 			elog(ERROR, "cache lookup failed for relation %u", relid);
! 		
! 
! 		heap_endscan(scan);
! 
! 		heap_close(relation, AccessShareLock);
! 
! 	}
  	relform = (Form_pg_class) GETSTRUCT(reltup);
  
  	recomputeNamespacePath();
***************
*** 446,452 ****
  		}
  	}
  
! 	ReleaseSysCache(reltup);
  
  	return visible;
  }
--- 478,487 ----
  		}
  	}
  
! 	if (fromcache)
! 		ReleaseSysCache(reltup);
! 	else
! 		heap_freetuple(reltup);
  
  	return visible;
  }
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Kirkwood (#9)
Re: [HACKERS] Cache lookup failed for relation X [was: DROP FUNCTION cache lookup failed for relation X]

Mark Kirkwood <markir@paradise.net.nz> writes:

The previous discussion centered around working on on locking in
dependency.c whilst dropping related objects - but does this apply when
there is just one? Anyway I tried to understand what was happening and
the attached rather hacky patch seems to cure the behaviour - So I've
submitted it as a discussion aid, rather than 'the way of fixing
this'... since I'm hoping there is a better way :-)

AFAICS this just makes the window a bit narrower.

regards, tom lane