Temp tables, pg_class_temp and AccessExclusiveLocks
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
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);
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 trafficWhy 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 managerMy 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