From df383b03e6f761c9b5bb12aa2339795ab44aa054 Mon Sep 17 00:00:00 2001 From: Simon Riggs Date: Wed, 7 Sep 2011 12:11:26 +0100 Subject: [PATCH] Partially revoke attempt to improve performance with many savepoints. Maintain difference between subtransaction release and commit introduced by earlier patch. --- src/backend/access/transam/xact.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 59685dc85c..7c0b463067 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -269,7 +269,7 @@ static TransactionId RecordTransactionAbort(bool isSubXact); static void StartTransaction(void); static void StartSubTransaction(void); -static void CommitSubTransaction(bool isTopLevel); +static void CommitSubTransaction(void); static void AbortSubTransaction(void); static void CleanupSubTransaction(void); static void PushTransaction(void); @@ -2578,7 +2578,7 @@ CommitTransactionCommand(void) case TBLOCK_SUBRELEASE: do { - CommitSubTransaction(false); + CommitSubTransaction(); s = CurrentTransactionState; /* changed by pop */ } while (s->blockState == TBLOCK_SUBRELEASE); @@ -2588,12 +2588,17 @@ CommitTransactionCommand(void) /* * We were issued a COMMIT, so we end the current subtransaction - * hierarchy and perform final commit. + * hierarchy and perform final commit. We do this by rolling up + * any subtransactions into their parent, which leads to O(N^2) + * operations with respect to resource owners - this isn't that + * bad until we approach a thousands of savepoints but is necessary + * for correctness should after triggers create new resource + * owners. */ case TBLOCK_SUBCOMMIT: do { - CommitSubTransaction(true); + CommitSubTransaction(); s = CurrentTransactionState; /* changed by pop */ } while (s->blockState == TBLOCK_SUBCOMMIT); /* If we had a COMMIT command, finish off the main xact too */ @@ -3745,7 +3750,7 @@ ReleaseCurrentSubTransaction(void) BlockStateAsString(s->blockState)); Assert(s->state == TRANS_INPROGRESS); MemoryContextSwitchTo(CurTransactionContext); - CommitSubTransaction(false); + CommitSubTransaction(); s = CurrentTransactionState; /* changed by pop */ Assert(s->state == TRANS_INPROGRESS); } @@ -4009,13 +4014,9 @@ StartSubTransaction(void) * * The caller has to make sure to always reassign CurrentTransactionState * if it has a local pointer to it after calling this function. - * - * isTopLevel means that this CommitSubTransaction() is being issued as a - * sequence of actions leading directly to a main transaction commit - * allowing some actions to be optimised. */ static void -CommitSubTransaction(bool isTopLevel) +CommitSubTransaction(void) { TransactionState s = CurrentTransactionState; @@ -4069,14 +4070,10 @@ CommitSubTransaction(bool isTopLevel) /* * Other locks should get transferred to their parent resource owner. - * Doing that is an O(N^2) operation, so if isTopLevel then we can just - * leave the lock records as they are, knowing they will all get released - * by the top level commit using ProcReleaseLocks(). We only optimize - * this for commit; aborts may need to do other cleanup. */ ResourceOwnerRelease(s->curTransactionOwner, RESOURCE_RELEASE_LOCKS, - true, isTopLevel); + true, false); ResourceOwnerRelease(s->curTransactionOwner, RESOURCE_RELEASE_AFTER_LOCKS, true, false); -- 2.40.0