Index: src/backend/access/transam/transam.c =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/transam.c,v retrieving revision 1.76 diff -c -r1.76 transam.c *** src/backend/access/transam/transam.c 26 Mar 2008 18:48:59 -0000 1.76 --- src/backend/access/transam/transam.c 16 Sep 2008 16:55:30 -0000 *************** *** 287,317 **** return false; } - - /* - * TransactionIdCommit - * Commits the transaction associated with the identifier. - * - * Note: - * Assumes transaction identifier is valid. - */ - void - TransactionIdCommit(TransactionId transactionId) - { - TransactionLogUpdate(transactionId, TRANSACTION_STATUS_COMMITTED, - InvalidXLogRecPtr); - } - - /* - * TransactionIdAsyncCommit - * Same as above, but for async commits. The commit record LSN is needed. - */ - void - TransactionIdAsyncCommit(TransactionId transactionId, XLogRecPtr lsn) - { - TransactionLogUpdate(transactionId, TRANSACTION_STATUS_COMMITTED, lsn); - } - /* * TransactionIdAbort * Aborts the transaction associated with the identifier. --- 287,292 ---- *************** *** 328,359 **** } /* - * TransactionIdSubCommit - * Marks the subtransaction associated with the identifier as - * sub-committed. - * - * Note: - * No async version of this is needed. - */ - void - TransactionIdSubCommit(TransactionId transactionId) - { - TransactionLogUpdate(transactionId, TRANSACTION_STATUS_SUB_COMMITTED, - InvalidXLogRecPtr); - } - - /* * TransactionIdCommitTree * Marks all the given transaction ids as committed. * * The caller has to be sure that this is used only to mark subcommitted * subtransactions as committed, and only *after* marking the toplevel * parent as committed. Otherwise there is a race condition against ! * TransactionIdDidCommit. */ void ! TransactionIdCommitTree(int nxids, TransactionId *xids) { if (nxids > 0) TransactionLogMultiUpdate(nxids, xids, TRANSACTION_STATUS_COMMITTED, InvalidXLogRecPtr); --- 303,335 ---- } /* * TransactionIdCommitTree * Marks all the given transaction ids as committed. * * The caller has to be sure that this is used only to mark subcommitted * subtransactions as committed, and only *after* marking the toplevel * parent as committed. Otherwise there is a race condition against ! * TransactionIdDidCommit since we do not apply changes atomically, yet. */ void ! TransactionIdCommitTree(TransactionId commitxid, int nxids, TransactionId *xids) { + /* + * Mark top level transaction id as committed first, to avoid + * race conditions with TransactionIdDidCommit + */ + TransactionLogUpdate(commitxid, TRANSACTION_STATUS_COMMITTED, + InvalidXLogRecPtr); + + /* + * If there is more than one subcommit, then we need to mark them + * subcommitted first to ensure there is no race condition where + * we might see a subtransaction as still in progress when it is + * now committed. + */ + if (nxids > 1) + TransactionLogMultiUpdate(nxids, xids, TRANSACTION_STATUS_SUB_COMMITTED, + InvalidXLogRecPtr); if (nxids > 0) TransactionLogMultiUpdate(nxids, xids, TRANSACTION_STATUS_COMMITTED, InvalidXLogRecPtr); *************** *** 364,371 **** * Same as above, but for async commits. The commit record LSN is needed. */ void ! TransactionIdAsyncCommitTree(int nxids, TransactionId *xids, XLogRecPtr lsn) { if (nxids > 0) TransactionLogMultiUpdate(nxids, xids, TRANSACTION_STATUS_COMMITTED, lsn); --- 340,363 ---- * Same as above, but for async commits. The commit record LSN is needed. */ void ! TransactionIdAsyncCommitTree(TransactionId commitxid, int nxids, TransactionId *xids, XLogRecPtr lsn) { + /* + * Mark top level transaction id as committed first, to avoid + * race conditions with TransactionIdDidCommit + */ + TransactionLogUpdate(commitxid, TRANSACTION_STATUS_COMMITTED, + lsn); + + /* + * If there is more than one subcommit, then we need to mark them + * subcommitted first to ensure there is no race condition where + * we might see a subtransaction as still in progress when it is + * now committed. + */ + if (nxids > 1) + TransactionLogMultiUpdate(nxids, xids, TRANSACTION_STATUS_SUB_COMMITTED, + lsn); if (nxids > 0) TransactionLogMultiUpdate(nxids, xids, TRANSACTION_STATUS_COMMITTED, lsn); Index: src/backend/access/transam/twophase.c =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/twophase.c,v retrieving revision 1.45 diff -c -r1.45 twophase.c *** src/backend/access/transam/twophase.c 11 Aug 2008 11:05:10 -0000 1.45 --- src/backend/access/transam/twophase.c 16 Sep 2008 16:47:21 -0000 *************** *** 1745,1753 **** XLogFlush(recptr); /* Mark the transaction committed in pg_clog */ ! TransactionIdCommit(xid); ! /* to avoid race conditions, the parent must commit first */ ! TransactionIdCommitTree(nchildren, children); /* Checkpoint can proceed now */ MyProc->inCommit = false; --- 1745,1751 ---- XLogFlush(recptr); /* Mark the transaction committed in pg_clog */ ! TransactionIdCommitTree(xid, nchildren, children); /* Checkpoint can proceed now */ MyProc->inCommit = false; Index: src/backend/access/transam/xact.c =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/access/transam/xact.c,v retrieving revision 1.265 diff -c -r1.265 xact.c *** src/backend/access/transam/xact.c 11 Aug 2008 11:05:10 -0000 1.265 --- src/backend/access/transam/xact.c 16 Sep 2008 16:40:17 -0000 *************** *** 254,260 **** static TransactionId RecordTransactionAbort(bool isSubXact); static void StartTransaction(void); - static void RecordSubTransactionCommit(void); static void StartSubTransaction(void); static void CommitSubTransaction(void); static void AbortSubTransaction(void); --- 254,259 ---- *************** *** 952,962 **** * Now we may update the CLOG, if we wrote a COMMIT record above */ if (markXidCommitted) ! { ! TransactionIdCommit(xid); ! /* to avoid race conditions, the parent must commit first */ ! TransactionIdCommitTree(nchildren, children); ! } } else { --- 951,957 ---- * Now we may update the CLOG, if we wrote a COMMIT record above */ if (markXidCommitted) ! TransactionIdCommitTree(xid, nchildren, children); } else { *************** *** 974,984 **** * flushed before the CLOG may be updated. */ if (markXidCommitted) ! { ! TransactionIdAsyncCommit(xid, XactLastRecEnd); ! /* to avoid race conditions, the parent must commit first */ ! TransactionIdAsyncCommitTree(nchildren, children, XactLastRecEnd); ! } } /* --- 969,975 ---- * flushed before the CLOG may be updated. */ if (markXidCommitted) ! TransactionIdAsyncCommitTree(xid, nchildren, children, XactLastRecEnd); } /* *************** *** 1156,1191 **** s->maxChildXids = 0; } - /* - * RecordSubTransactionCommit - */ - static void - RecordSubTransactionCommit(void) - { - TransactionId xid = GetCurrentTransactionIdIfAny(); - - /* - * We do not log the subcommit in XLOG; it doesn't matter until the - * top-level transaction commits. - * - * We must mark the subtransaction subcommitted in the CLOG if it had a - * valid XID assigned. If it did not, nobody else will ever know about - * the existence of this subxact. We don't have to deal with deletions - * scheduled for on-commit here, since they'll be reassigned to our parent - * (who might still abort). - */ - if (TransactionIdIsValid(xid)) - { - /* XXX does this really need to be a critical section? */ - START_CRIT_SECTION(); - - /* Record subtransaction subcommit */ - TransactionIdSubCommit(xid); - - END_CRIT_SECTION(); - } - } - /* ---------------------------------------------------------------- * AbortTransaction stuff * ---------------------------------------------------------------- --- 1147,1152 ---- *************** *** 3791,3799 **** /* Must CCI to ensure commands of subtransaction are seen as done */ CommandCounterIncrement(); - /* Mark subtransaction as subcommitted */ - RecordSubTransactionCommit(); - /* Post-commit cleanup */ if (TransactionIdIsValid(s->transactionId)) AtSubCommit_childXids(); --- 3752,3757 ---- *************** *** 4259,4269 **** TransactionId max_xid; int i; - TransactionIdCommit(xid); - /* Mark committed subtransactions as committed */ sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]); ! TransactionIdCommitTree(xlrec->nsubxacts, sub_xids); /* Make sure nextXid is beyond any XID mentioned in the record */ max_xid = xid; --- 4217,4225 ---- TransactionId max_xid; int i; /* Mark committed subtransactions as committed */ sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]); ! TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids); /* Make sure nextXid is beyond any XID mentioned in the record */ max_xid = xid; Index: src/include/access/transam.h =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/include/access/transam.h,v retrieving revision 1.65 diff -c -r1.65 transam.h *** src/include/access/transam.h 11 Mar 2008 20:20:35 -0000 1.65 --- src/include/access/transam.h 16 Sep 2008 17:05:12 -0000 *************** *** 139,150 **** extern bool TransactionIdDidCommit(TransactionId transactionId); extern bool TransactionIdDidAbort(TransactionId transactionId); extern bool TransactionIdIsKnownCompleted(TransactionId transactionId); - extern void TransactionIdCommit(TransactionId transactionId); - extern void TransactionIdAsyncCommit(TransactionId transactionId, XLogRecPtr lsn); extern void TransactionIdAbort(TransactionId transactionId); ! extern void TransactionIdSubCommit(TransactionId transactionId); ! extern void TransactionIdCommitTree(int nxids, TransactionId *xids); ! extern void TransactionIdAsyncCommitTree(int nxids, TransactionId *xids, XLogRecPtr lsn); extern void TransactionIdAbortTree(int nxids, TransactionId *xids); extern bool TransactionIdPrecedes(TransactionId id1, TransactionId id2); extern bool TransactionIdPrecedesOrEquals(TransactionId id1, TransactionId id2); --- 139,147 ---- extern bool TransactionIdDidCommit(TransactionId transactionId); extern bool TransactionIdDidAbort(TransactionId transactionId); extern bool TransactionIdIsKnownCompleted(TransactionId transactionId); extern void TransactionIdAbort(TransactionId transactionId); ! extern void TransactionIdCommitTree(TransactionId commitxid, int nxids, TransactionId *xids); ! extern void TransactionIdAsyncCommitTree(TransactionId commitxid, int nxids, TransactionId *xids, XLogRecPtr lsn); extern void TransactionIdAbortTree(int nxids, TransactionId *xids); extern bool TransactionIdPrecedes(TransactionId id1, TransactionId id2); extern bool TransactionIdPrecedesOrEquals(TransactionId id1, TransactionId id2);