Temp tables, pg_class_temp and AccessExclusiveLocks

Started by Simon Riggsabout 11 years ago3 messages
#1Simon Riggs
simon@2ndQuadrant.com

While investigating how to reduce logging of AccessExclusiveLocks for
temp tables, I came up with the attached patch.

My feeling was "that's an ugly patch", but it set me thinking that a
more specialised code path around temp tables could be useful in other
ways, and therefore worth pursuing further.

The patch creates a relation_open_temp(), which could then allow a
RelationIdGetTempRelation() which could have a path that calls
ScanPgRelation on a new catalog table called pg_class_temp, and other
_temp catalog table accesses.

So we could isolate all temp table access to specific tables. That would
* cause less bloat in the main catalog tables
* avoid logging AccessExclusiveLocks on temp tables
* less WAL traffic

Why less WAL traffic? Because we can choose then to make *_temp catalog tables
1. use unlogged table mechanism
2. use local tables inside the local buffer manager

My preference would be (1) because all we need to do is change the
catalog table oid when searching, we don't need to do anything else.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Simon Riggs
simon@2ndQuadrant.com
In reply to: Simon Riggs (#1)
1 attachment(s)
Re: Temp tables, pg_class_temp and AccessExclusiveLocks

On 31 October 2014 14:53, Simon Riggs <simon@2ndquadrant.com> wrote:

While investigating how to reduce logging of AccessExclusiveLocks for
temp tables, I came up with the attached patch.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

temp_tables_skip_logging_locks.v1.patchapplication/octet-stream; name=temp_tables_skip_logging_locks.v1.patchDownload
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index ae15c0b..cda51cc 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1014,6 +1014,8 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
  *
  *		NB: a "relation" is anything with a pg_class entry.  The caller is
  *		expected to check whether the relkind is something it can handle.
+ *
+ *		relation_open_temp() is a special case that avoids logging locks.
  * ----------------
  */
 Relation
@@ -1042,6 +1044,31 @@ relation_open(Oid relationId, LOCKMODE lockmode)
 	return r;
 }
 
+Relation
+relation_open_temp(Oid relationId, LOCKMODE lockmode)
+{
+	Relation	r;
+
+	Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES);
+
+	/* Get the lock before trying to open the relcache entry */
+	if (lockmode != NoLock)
+		LockRelationOidTemp(relationId, lockmode);
+
+	/* The relcache does all the real work... */
+	r = RelationIdGetRelation(relationId);
+
+	if (!RelationIsValid(r))
+		elog(ERROR, "could not open relation with OID %u", relationId);
+
+	/* Make note that we've accessed a temporary relation */
+	MyXactAccessedTempRel = true;
+
+	pgstat_initstats(r);
+
+	return r;
+}
+
 /* ----------------
  *		try_relation_open - open any relation by relation OID
  *
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7bc579b..f525a47 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -669,8 +669,13 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
 	 * really necessary for locking out other backends (since they can't see
 	 * the new rel anyway until we commit), but it keeps the lock manager from
 	 * complaining about deadlock risks.
+	 *
+	 * Temp tables follow a special case here to avoid logging locks.
 	 */
-	rel = relation_open(relationId, AccessExclusiveLock);
+	if (stmt->relation->relpersistence == RELPERSISTENCE_TEMP)
+		rel = relation_open_temp(relationId, AccessExclusiveLock);
+	else
+		rel = relation_open(relationId, AccessExclusiveLock);
 
 	/*
 	 * Now add any newly specified column default values and CHECK constraints
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 1c327fd..2ed545a 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -365,7 +365,7 @@ ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
 		ResolveRecoveryConflictWithVirtualXIDs(backends,
 											 PROCSIG_RECOVERY_CONFLICT_LOCK);
 
-		if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false)
+		if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false, false)
 			!= LOCKACQUIRE_NOT_AVAIL)
 			lock_acquired = true;
 	}
@@ -591,7 +591,7 @@ StandbyAcquireAccessExclusiveLock(TransactionId xid, Oid dbOid, Oid relOid)
 	 */
 	SET_LOCKTAG_RELATION(locktag, newlock->dbOid, newlock->relOid);
 
-	if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false)
+	if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false, false)
 		== LOCKACQUIRE_NOT_AVAIL)
 		ResolveRecoveryConflictWithLock(newlock->dbOid, newlock->relOid);
 }
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 6cc4d26..747f0ac 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -82,6 +82,7 @@ SetLocktagRelationOid(LOCKTAG *tag, Oid relid)
  *
  * Lock a relation given only its OID.  This should generally be used
  * before attempting to open the relation's relcache entry.
+ * Any changes here should also change LockRelationOidTemp below.
  */
 void
 LockRelationOid(Oid relid, LOCKMODE lockmode)
@@ -108,6 +109,38 @@ LockRelationOid(Oid relid, LOCKMODE lockmode)
 }
 
 /*
+ *		LockRelationOidTemp
+ *
+ * Lock a relation given only its OID.  This should generally be used
+ * before attempting to open the relation's relcache entry.
+ *
+ * Special case version of LockRelationOid that avoids logging locks.
+ */
+void
+LockRelationOidTemp(Oid relid, LOCKMODE lockmode)
+{
+	LOCKTAG		tag;
+	LockAcquireResult res;
+
+	SetLocktagRelationOid(&tag, relid);
+
+	res = LockAcquireExtended(&tag, lockmode, false, false, true, false);
+
+	/*
+	 * Now that we have the lock, check for invalidation messages, so that we
+	 * will update or flush any stale relcache entry before we try to use it.
+	 * RangeVarGetRelid() specifically relies on us for this.  We can skip
+	 * this in the not-uncommon case that we already had the same type of lock
+	 * being requested, since then no one else could have modified the
+	 * relcache entry in an undesirable way.  (In the case where our own xact
+	 * modifies the rel, the relcache update happens via
+	 * CommandCounterIncrement, not here.)
+	 */
+	if (res != LOCKACQUIRE_ALREADY_HELD)
+		AcceptInvalidationMessages();
+}
+
+/*
  *		ConditionalLockRelationOid
  *
  * As above, but only lock if we can get the lock without blocking.
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 723051e..e281870 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -674,7 +674,7 @@ LockAcquire(const LOCKTAG *locktag,
 			bool sessionLock,
 			bool dontWait)
 {
-	return LockAcquireExtended(locktag, lockmode, sessionLock, dontWait, true);
+	return LockAcquireExtended(locktag, lockmode, sessionLock, dontWait, true, true);
 }
 
 /*
@@ -691,7 +691,8 @@ LockAcquireExtended(const LOCKTAG *locktag,
 					LOCKMODE lockmode,
 					bool sessionLock,
 					bool dontWait,
-					bool reportMemoryError)
+					bool reportMemoryError,
+					bool log_lock)
 {
 	LOCKMETHODID lockmethodid = locktag->locktag_lockmethodid;
 	LockMethod	lockMethodTable;
@@ -704,7 +705,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
 	uint32		hashcode;
 	LWLock	   *partitionLock;
 	int			status;
-	bool		log_lock = false;
+	bool		lock_logged = false;
 
 	if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
 		elog(ERROR, "unrecognized lock method: %d", lockmethodid);
@@ -801,11 +802,11 @@ LockAcquireExtended(const LOCKTAG *locktag,
 	 */
 	if (lockmode >= AccessExclusiveLock &&
 		locktag->locktag_type == LOCKTAG_RELATION &&
-		!RecoveryInProgress() &&
+		log_lock &&
 		XLogStandbyInfoActive())
 	{
 		LogAccessExclusiveLockPrepare();
-		log_lock = true;
+		lock_logged = true;
 	}
 
 	/*
@@ -1028,7 +1029,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
 	 * Emit a WAL record if acquisition of this lock needs to be replayed in a
 	 * standby server.
 	 */
-	if (log_lock)
+	if (lock_logged)
 	{
 		/*
 		 * Decode the locktag back to the original values, to avoid sending
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 493839f..97b3502 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -81,6 +81,7 @@ typedef struct HeapUpdateFailureData
 
 /* in heap/heapam.c */
 extern Relation relation_open(Oid relationId, LOCKMODE lockmode);
+extern Relation relation_open_temp(Oid relationId, LOCKMODE lockmode);
 extern Relation try_relation_open(Oid relationId, LOCKMODE lockmode);
 extern Relation relation_openrv(const RangeVar *relation, LOCKMODE lockmode);
 extern Relation relation_openrv_extended(const RangeVar *relation,
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index e9447b7..52bcbe1 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -38,6 +38,7 @@ extern void RelationInitLockInfo(Relation relation);
 
 /* Lock a relation */
 extern void LockRelationOid(Oid relid, LOCKMODE lockmode);
+extern void LockRelationOidTemp(Oid relid, LOCKMODE lockmode);
 extern bool ConditionalLockRelationOid(Oid relid, LOCKMODE lockmode);
 extern void UnlockRelationId(LockRelId *relid, LOCKMODE lockmode);
 extern void UnlockRelationOid(Oid relid, LOCKMODE lockmode);
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 4c49e3c..75e7797 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -503,7 +503,8 @@ extern LockAcquireResult LockAcquireExtended(const LOCKTAG *locktag,
 					LOCKMODE lockmode,
 					bool sessionLock,
 					bool dontWait,
-					bool report_memory_error);
+					bool report_memory_error,
+					bool log_lock);
 extern void AbortStrongLockAcquire(void);
 extern bool LockRelease(const LOCKTAG *locktag,
 			LOCKMODE lockmode, bool sessionLock);
#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Simon Riggs (#1)
Re: Temp tables, pg_class_temp and AccessExclusiveLocks

Hi

good idea. Mix of these both strategies can be a base for global temp
tables implementation.

Pavel

2014-10-31 15:53 GMT+01:00 Simon Riggs <simon@2ndquadrant.com>:

Show quoted text

While investigating how to reduce logging of AccessExclusiveLocks for
temp tables, I came up with the attached patch.

My feeling was "that's an ugly patch", but it set me thinking that a
more specialised code path around temp tables could be useful in other
ways, and therefore worth pursuing further.

The patch creates a relation_open_temp(), which could then allow a
RelationIdGetTempRelation() which could have a path that calls
ScanPgRelation on a new catalog table called pg_class_temp, and other
_temp catalog table accesses.

So we could isolate all temp table access to specific tables. That would
* cause less bloat in the main catalog tables
* avoid logging AccessExclusiveLocks on temp tables
* less WAL traffic

Why less WAL traffic? Because we can choose then to make *_temp catalog
tables
1. use unlogged table mechanism
2. use local tables inside the local buffer manager

My preference would be (1) because all we need to do is change the
catalog table oid when searching, we don't need to do anything else.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers