On 05/06/2014 02:44 PM, Andres Freund wrote:
On 2014-05-05 13:41:00 +0300, Heikki Linnakangas wrote:+/* + * Exit hook to unlock the global transaction entry we're working on. + */ +static void +AtProcExit_Twophase(int code, Datum arg) +{ + /* same logic as abort */ + AtAbort_Twophase(); +} + +/* + * Abort hook to unlock the global transaction entry we're working on. + */ +void +AtAbort_Twophase(void) +{ + if (MyLockedGxact == NULL) + return; + + /* + * If we were in process of preparing the transaction, but haven't + * written the WAL record yet, remove the global transaction entry. + * Same if we are in the process of finishing an already-prepared + * transaction, and fail after having already written the WAL 2nd + * phase commit or rollback record. + * + * After that it's too late to abort, so just unlock the GlobalTransaction + * entry. We might not have transfered all locks and other state to the + * prepared transaction yet, so this is a bit bogus, but it's the best we + * can do. + */ + if (!MyLockedGxact->valid) + { + RemoveGXact(MyLockedGxact); + } + else + { + LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE); + + MyLockedGxact->locking_backend = InvalidBackendId; + + LWLockRelease(TwoPhaseStateLock); + } + MyLockedGxact = NULL; +}Is it guaranteed that all paths have called LWLockReleaseAll() before calling the proc exit hooks? Otherwise we might end up waiting for ourselves...
Hmm. AbortTransaction() will release locks before we get here, but the before_shmem_exit() callpath will not. So an elog(FATAL), while holding TwoPhaseStateLock would cause us to deadlock with ourself. But there are no such elogs.
I copied this design from async.c, which is quite similar, so if there's a problem that ought to be fixed too. And there are other more complicated before_shmem callbacks that worry me more, like createdb_failure_callback(). But I think they're all all right.
/* * MarkAsPreparing @@ -261,29 +329,15 @@ MarkAsPreparing(TransactionId xid, const char *gid, errmsg("prepared transactions are disabled"), errhint("Set max_prepared_transactions to a nonzero value."))); - LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE); - - /* - * First, find and recycle any gxacts that failed during prepare. We do - * this partly to ensure we don't mistakenly say their GIDs are still - * reserved, and partly so we don't fail on out-of-slots unnecessarily. - */ - for (i = 0; i < TwoPhaseState->numPrepXacts; i++) + /* on first call, register the exit hook */ + if (!twophaseExitRegistered) { - gxact = TwoPhaseState->prepXacts[i]; - if (!gxact->valid && !TransactionIdIsActive(gxact->locking_xid)) - { - /* It's dead Jim ... remove from the active array */ - TwoPhaseState->numPrepXacts--; - TwoPhaseState->prepXacts[i] = TwoPhaseState->prepXacts[TwoPhaseState->numPrepXacts]; - /* and put it back in the freelist */ - gxact->next = TwoPhaseState->freeGXacts; - TwoPhaseState->freeGXacts = gxact; - /* Back up index count too, so we don't miss scanning one */ - i--; - } + before_shmem_exit(AtProcExit_Twophase, 0); + twophaseExitRegistered = true; }It's not particularly nice to register shmem exit hooks in the middle of normal processing because it makes it impossible to use cancel_before_shmem_exit() previously registered hooks. I think this should be registered at startup, if max_prepared_xacts > 0.
<shrug>. async.c and namespace.c does the same, and it hasn't been a problem.
I committed this now, but please let me know if you see a concrete problem with the locks.
- Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
