Re: "could not open relation 1663/16384/16584: No such file or directory" in a specific combination of transactions with temp tables

Tom Lane wrote:
> "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
>> Tom Lane wrote:
>>> Actually ... why are we using the lock manager to drive this at all?
>
>> Good question. It has always seemed a bit strange to me. The assumption
>> that we always hold the lock on temp table until end of transaction,
>> while true today, seems weak to me.
>
> Looking back, I think it was driven by the desire to tie the behavior
> directly to things that are going to get persisted, such as locks.
> From that standpoint your initial patch to attach a temp-check to
> relation-drop 2PC entries would be the right kind of design.  However,
> given what we now know about the lock situation, I'd feel uncomfortable
> with applying that without also fixing LockTagIsTemp, and right now
> that's looking like much more complexity and possible performance
> penalty than it's worth.

Looking closer, this actually worked in 8.1, and was broken in 8.2 by
this change:

> date: 2006-07-31 21:09:05 +0100;  author: tgl;  state: Exp;  lines: +167 -48;
> Change the relation_open protocol so that we obtain lock on a relation
> (table or index) before trying to open its relcache entry.  This fixes
> race conditions in which someone else commits a change to the relation's
> catalog entries while we are in process of doing relcache load.  Problems
> of that ilk have been reported sporadically for years, but it was not
> really practical to fix until recently --- for instance, the recent
> addition of WAL-log support for in-place updates helped.
>
> Along the way, remove pg_am.amconcurrent: all AMs are now expected to support
> concurrent update.

Before that, we had an isTempObject flag in LOCALLOCK, which worked even
when the relation was dropped later on, unlike LockTagIsTemp.

Anyway, patches attached, using the global flag approach, for 8.2 and
8.3. As discussed earlier, since the flag is global, we won't allow
PREPARE TRANSACTION if you have operated on a temp table in an aborted
subxact, but I think that's acceptable.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.249
diff -c -r1.249 heapam.c
*** src/backend/access/heap/heapam.c    30 Jan 2008 18:35:55 -0000    1.249
--- src/backend/access/heap/heapam.c    4 Mar 2008 12:43:25 -0000
***************
*** 868,873 ****
--- 868,877 ----
      if (!RelationIsValid(r))
          elog(ERROR, "could not open relation with OID %u", relationId);

+     /* Make note that we've accessed a temporary relation. */
+     if (r->rd_istemp)
+         MyXactAccessedTempRel = true;
+
      pgstat_initstats(r);

      return r;
***************
*** 912,917 ****
--- 916,925 ----
      if (!RelationIsValid(r))
          elog(ERROR, "could not open relation with OID %u", relationId);

+     /* Make note that we've accessed a temporary relation. */
+     if (r->rd_istemp)
+         MyXactAccessedTempRel = true;
+
      pgstat_initstats(r);

      return r;
***************
*** 958,963 ****
--- 966,975 ----
      if (!RelationIsValid(r))
          elog(ERROR, "could not open relation with OID %u", relationId);

+     /* Make note that we've accessed a temporary relation. */
+     if (r->rd_istemp)
+         MyXactAccessedTempRel = true;
+
      pgstat_initstats(r);

      return r;
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257
diff -c -r1.257 xact.c
*** src/backend/access/transam/xact.c    15 Jan 2008 18:56:59 -0000    1.257
--- src/backend/access/transam/xact.c    4 Mar 2008 12:29:21 -0000
***************
*** 189,194 ****
--- 189,200 ----
  static bool forceSyncCommit = false;

  /*
+  * MyXactAccessedTempRel is set when a temporary relation is accessed. We
+  * don't allow PREPARE TRANSACTION in that case.
+  */
+ bool MyXactAccessedTempRel = false;
+
+ /*
   * Private context for transaction-abort work --- we reserve space for this
   * at startup to ensure that AbortTransaction and AbortSubTransaction can work
   * when we've run out of memory.
***************
*** 1445,1450 ****
--- 1451,1457 ----
      XactIsoLevel = DefaultXactIsoLevel;
      XactReadOnly = DefaultXactReadOnly;
      forceSyncCommit = false;
+     MyXactAccessedTempRel = false;

      /*
       * reinitialize within-transaction counters
***************
*** 1770,1775 ****
--- 1777,1808 ----

      /* NOTIFY and flatfiles will be handled below */

+     /*
+      * Don't allow PREPARE TRANSACTION if we've accessed a temporary table
+      * in this transaction. It's not clear what should happen if a prepared
+      * transaction holds a lock on a temp table, and the original backend
+      * exits and deletes the file, for example. Also, if a temp table is
+      * dropped, we have no way of flushing temp buffers from the backend-
+      * private temp buffer cache of the original backend at COMMIT PREPARED,
+      * since it can be executed in a different backend.
+      *
+      * We need to check this after executing any ON COMMIT actions, because
+      * they might still access a temp relation.
+      *
+      * It would be nice to relax this restriction so that you could operate on
+      * ON COMMIT DELETE ROWS temp tables, or on one created and dropped in the
+      * same transaction, by releasing the locks on it at PREPARE TRANSACTION,
+      * instead of COMMIT PREPARED as usual. Another case we could allow is if
+      * the access to the temp relation happened in a subtransaction that later
+      * rolled back. MyXactAccessedTempRel would needto be per-subxact to track
+      * that, but it doesn't seem worth the effort given how narrow the use
+      * case for that is.
+      */
+     if (MyXactAccessedTempRel)
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+
      /* Prevent cancel/die interrupt while cleaning up */
      HOLD_INTERRUPTS();

Index: src/backend/storage/lmgr/lmgr.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/storage/lmgr/lmgr.c,v
retrieving revision 1.96
diff -c -r1.96 lmgr.c
*** src/backend/storage/lmgr/lmgr.c    8 Jan 2008 23:18:50 -0000    1.96
--- src/backend/storage/lmgr/lmgr.c    4 Mar 2008 12:13:28 -0000
***************
*** 662,706 ****
      LockRelease(&tag, lockmode, false);
  }

-
- /*
-  * LockTagIsTemp
-  *        Determine whether a locktag is for a lock on a temporary object
-  *
-  * We need this because 2PC cannot deal with temp objects
-  */
- bool
- LockTagIsTemp(const LOCKTAG *tag)
- {
-     switch ((LockTagType) tag->locktag_type)
-     {
-         case LOCKTAG_RELATION:
-         case LOCKTAG_RELATION_EXTEND:
-         case LOCKTAG_PAGE:
-         case LOCKTAG_TUPLE:
-             /* check for lock on a temp relation */
-             /* field1 is dboid, field2 is reloid for all of these */
-             if ((Oid) tag->locktag_field1 == InvalidOid)
-                 return false;    /* shared, so not temp */
-             if (isTempOrToastNamespace(get_rel_namespace((Oid) tag->locktag_field2)))
-                 return true;
-             break;
-         case LOCKTAG_TRANSACTION:
-         case LOCKTAG_VIRTUALTRANSACTION:
-             /* there are no temp transactions */
-             break;
-         case LOCKTAG_OBJECT:
-             /* there are currently no non-table temp objects */
-             break;
-         case LOCKTAG_USERLOCK:
-         case LOCKTAG_ADVISORY:
-             /* assume these aren't temp */
-             break;
-     }
-     return false;                /* default case */
- }
-
-
  /*
   * Append a description of a lockable object to buf.
   *
--- 662,667 ----
Index: src/backend/storage/lmgr/lock.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.181
diff -c -r1.181 lock.c
*** src/backend/storage/lmgr/lock.c    2 Feb 2008 22:26:17 -0000    1.181
--- src/backend/storage/lmgr/lock.c    4 Mar 2008 12:13:09 -0000
***************
*** 1871,1882 ****
                  elog(ERROR, "cannot PREPARE when session locks exist");
          }

-         /* Can't handle it if the lock is on a temporary object */
-         if (LockTagIsTemp(&locallock->tag.lock))
-             ereport(ERROR,
-                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                      errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
-
          /*
           * Create a 2PC record.
           */
--- 1871,1876 ----
Index: src/include/access/xact.h
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/include/access/xact.h,v
retrieving revision 1.93
diff -c -r1.93 xact.h
*** src/include/access/xact.h    1 Jan 2008 19:45:56 -0000    1.93
--- src/include/access/xact.h    4 Mar 2008 12:27:12 -0000
***************
*** 44,49 ****
--- 44,51 ----
  /* Asynchronous commits */
  extern bool XactSyncCommit;

+ extern bool MyXactAccessedTempRel;
+
  /*
   *    start- and end-of-transaction callbacks for dynamically loaded modules
   */
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.222.2.1
diff -c -r1.222.2.1 heapam.c
*** src/backend/access/heap/heapam.c    4 Feb 2007 20:00:49 -0000    1.222.2.1
--- src/backend/access/heap/heapam.c    4 Mar 2008 12:42:05 -0000
***************
*** 699,704 ****
--- 699,708 ----
      if (!RelationIsValid(r))
          elog(ERROR, "could not open relation with OID %u", relationId);

+     /* Make note that we've accessed a temporary relation. */
+     if (r->rd_istemp)
+         MyXactAccessedTempRel = true;
+
      return r;
  }

***************
*** 741,746 ****
--- 745,755 ----
      if (!RelationIsValid(r))
          elog(ERROR, "could not open relation with OID %u", relationId);

+
+     /* Make note that we've accessed a temporary relation. */
+     if (r->rd_istemp)
+         MyXactAccessedTempRel = true;
+
      return r;
  }

***************
*** 785,790 ****
--- 794,803 ----
      if (!RelationIsValid(r))
          elog(ERROR, "could not open relation with OID %u", relationId);

+     /* Make note that we've accessed a temporary relation. */
+     if (r->rd_istemp)
+         MyXactAccessedTempRel = true;
+
      return r;
  }

Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.229.2.3
diff -c -r1.229.2.3 xact.c
*** src/backend/access/transam/xact.c    3 Jan 2008 21:23:45 -0000    1.229.2.3
--- src/backend/access/transam/xact.c    4 Mar 2008 12:34:43 -0000
***************
*** 174,179 ****
--- 174,185 ----
  static char *prepareGID;

  /*
+  * MyXactAccessedTempRel is set when a temporary relation is accessed. We
+  * don't allow PREPARE TRANSACTION in that case.
+  */
+ bool MyXactAccessedTempRel = false;
+
+ /*
   * Private context for transaction-abort work --- we reserve space for this
   * at startup to ensure that AbortTransaction and AbortSubTransaction can work
   * when we've run out of memory.
***************
*** 1389,1394 ****
--- 1395,1401 ----
      FreeXactSnapshot();
      XactIsoLevel = DefaultXactIsoLevel;
      XactReadOnly = DefaultXactReadOnly;
+     MyXactAccessedTempRel = false;

      /*
       * reinitialize within-transaction counters
***************
*** 1715,1720 ****
--- 1722,1753 ----

      /* NOTIFY and flatfiles will be handled below */

+     /*
+      * Don't allow PREPARE TRANSACTION if we've accessed a temporary table
+      * in this transaction. It's not clear what should happen if a prepared
+      * transaction holds a lock on a temp table, and the original backend
+      * exits and deletes the file, for example. Also, if a temp table is
+      * dropped, we have no way of flushing temp buffers from the backend-
+      * private temp buffer cache of the original backend at COMMIT PREPARED,
+      * since it can be executed in a different backend.
+      *
+      * We need to check this after executing any ON COMMIT actions, because
+      * they might still access a temp relation.
+      *
+      * It would be nice to relax this restriction so that you could operate on
+      * ON COMMIT DELETE ROWS temp tables, or on one created and dropped in the
+      * same transaction, by releasing the locks on it at PREPARE TRANSACTION,
+      * instead of COMMIT PREPARED as usual. Another case we could allow is if
+      * the access to the temp relation happened in a subtransaction that later
+      * rolled back. MyXactAccessedTempRel would needto be per-subxact to track
+      * that, but it doesn't seem worth the effort given how narrow the use
+      * case for that is.
+      */
+     if (MyXactAccessedTempRel)
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+
      /* Prevent cancel/die interrupt while cleaning up */
      HOLD_INTERRUPTS();

Index: src/backend/storage/lmgr/lmgr.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/storage/lmgr/lmgr.c,v
retrieving revision 1.89
diff -c -r1.89 lmgr.c
*** src/backend/storage/lmgr/lmgr.c    4 Oct 2006 00:29:57 -0000    1.89
--- src/backend/storage/lmgr/lmgr.c    4 Mar 2008 12:35:00 -0000
***************
*** 598,637 ****

      LockRelease(&tag, lockmode, false);
  }
-
-
- /*
-  * LockTagIsTemp
-  *        Determine whether a locktag is for a lock on a temporary object
-  *
-  * We need this because 2PC cannot deal with temp objects
-  */
- bool
- LockTagIsTemp(const LOCKTAG *tag)
- {
-     switch (tag->locktag_type)
-     {
-         case LOCKTAG_RELATION:
-         case LOCKTAG_RELATION_EXTEND:
-         case LOCKTAG_PAGE:
-         case LOCKTAG_TUPLE:
-             /* check for lock on a temp relation */
-             /* field1 is dboid, field2 is reloid for all of these */
-             if ((Oid) tag->locktag_field1 == InvalidOid)
-                 return false;    /* shared, so not temp */
-             if (isTempNamespace(get_rel_namespace((Oid) tag->locktag_field2)))
-                 return true;
-             break;
-         case LOCKTAG_TRANSACTION:
-             /* there are no temp transactions */
-             break;
-         case LOCKTAG_OBJECT:
-             /* there are currently no non-table temp objects */
-             break;
-         case LOCKTAG_USERLOCK:
-         case LOCKTAG_ADVISORY:
-             /* assume these aren't temp */
-             break;
-     }
-     return false;                /* default case */
- }
--- 598,600 ----
Index: src/backend/storage/lmgr/lock.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.174.2.1
diff -c -r1.174.2.1 lock.c
*** src/backend/storage/lmgr/lock.c    2 Feb 2008 22:26:23 -0000    1.174.2.1
--- src/backend/storage/lmgr/lock.c    4 Mar 2008 12:32:30 -0000
***************
*** 1849,1860 ****
                  elog(ERROR, "cannot PREPARE when session locks exist");
          }

-         /* Can't handle it if the lock is on a temporary object */
-         if (LockTagIsTemp(&locallock->tag.lock))
-             ereport(ERROR,
-                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                      errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
-
          /*
           * Create a 2PC record.
           */
--- 1849,1854 ----
Index: src/include/access/xact.h
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/include/access/xact.h,v
retrieving revision 1.83
diff -c -r1.83 xact.h
*** src/include/access/xact.h    11 Jul 2006 18:26:11 -0000    1.83
--- src/include/access/xact.h    4 Mar 2008 12:32:30 -0000
***************
*** 41,46 ****
--- 41,48 ----
  extern bool DefaultXactReadOnly;
  extern bool XactReadOnly;

+ extern bool MyXactAccessedTempRel;
+
  /*
   *    start- and end-of-transaction callbacks for dynamically loaded modules
   */