On Tue, 2008-09-16 at 10:11 -0400, Alvaro Herrera wrote:
> >> Right now we lock and unlock the clog for each committed subtransaction
> >> at commit time, which is wasteful. A better scheme:
> >> pre-scan the list of xids to derive list of pages
> >> if we have just a single page to update
> >> {
> >> update all entries on page in one action
> >> }
> >> else
> >> {
> >> loop thru xids marking them all as subcommitted
> >> mark top level transaction committed
> >> loop thus xids again marking them all as committed
> >> }
>
> > Hmm, I don't see anything immediately wrong with that.
>
> Neither do I.
>
> I wonder if the improved clog API required to mark multiple transactions
> as committed at once would be also useful to TransactionIdCommitTree
> which is used in regular transaction commit.
I enclose a patch to transform what we have now into what I think is
possible. If we agree this is possible, then I will do further work to
optimise transam.c (using clog.c changes also). So this is an
"intermediate" or precursor patch for discussion only.
backend/access/transam/transam.c | 78 ++++++++++++-----------------!
backend/access/transam/twophase.c | 4 !
backend/access/transam/xact.c | 50 -----------------!!!!!!!
include/access/transam.h | 7 !!!
4 files changed, 32 insertions(+), 78 deletions(-), 29 modifications(!)
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
