mirror/postgresql
2
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2024-12-21 08:29:39 +08:00
Code Issues Packages Projects Releases Wiki Activity

Move RegisterPredicateLockingXid() call to a safer place.

Browse Source 
The SSI patch inserted a call of RegisterPredicateLockingXid into
GetNewTransactionId, which was a bad idea on a couple of grounds.  First,
it's not necessary to hold XidGenLock while manipulating that shared
memory, and doing so is bad because XidGenLock is a high-contention lock
that should be held for as short a time as possible.  (Not to mention that
it adds an entirely unnecessary deadlock hazard, since we must take
SerializableXactHashLock as well.)  Second, the specific place where it was
put was between extending CLOG and advancing nextXid, which could result in
unpleasant behavior in case of a failure there.  Pull the call out to
AssignTransactionId, which is much safer and arguably better from a
modularity standpoint too.

There is more work to do to clean up the failure-before-advancing-nextXid
issue, but that is a separate change that will need to be back-patched.
So for the moment I just want to make GetNewTransactionId look the same as
it did in prior versions.
This commit is contained in:
Tom Lane 2011-05-06 12:57:28 -04:00
parent 12b7164578
commit d2088ae949
2 changed files with 7 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/backend/access/transam/varsup.c
View File

@ -21,7 +21,6 @@
#include "miscadmin.h" #include "miscadmin.h"
#include "postmaster/autovacuum.h" #include "postmaster/autovacuum.h"
#include "storage/pmsignal.h" #include "storage/pmsignal.h"
#include "storage/predicate.h"
#include "storage/proc.h" #include "storage/proc.h"
#include "utils/builtins.h" #include "utils/builtins.h"
#include "utils/syscache.h" #include "utils/syscache.h"
@ -162,10 +161,6 @@ GetNewTransactionId(bool isSubXact)
ExtendCLOG(xid); ExtendCLOG(xid);
ExtendSUBTRANS(xid); ExtendSUBTRANS(xid);
/* If it's top level, the predicate locking system also needs to know. */
if (!isSubXact)
RegisterPredicateLockingXid(xid);
/* /*
* Now advance the nextXid counter. This must not happen until after we * Now advance the nextXid counter. This must not happen until after we
* have successfully completed ExtendCLOG() --- if that routine fails, we * have successfully completed ExtendCLOG() --- if that routine fails, we

7
src/backend/access/transam/xact.c
View File

@ -454,6 +454,13 @@ AssignTransactionId(TransactionState s)
if (isSubXact) if (isSubXact)
SubTransSetParent(s->transactionId, s->parent->transactionId, false); SubTransSetParent(s->transactionId, s->parent->transactionId, false);
/*
* If it's a top-level transaction, the predicate locking system needs to
* be told about it too.
*/
if (!isSubXact)
RegisterPredicateLockingXid(s->transactionId);
/* /*
* Acquire lock on the transaction XID. (We assume this cannot block.) We * Acquire lock on the transaction XID. (We assume this cannot block.) We
* have to ensure that the lock is assigned to the transaction's own * have to ensure that the lock is assigned to the transaction's own