From: Simon Riggs <simon@2ndQuadrant.com>
To: Alvaro Herrera <alvherre@commandprompt.com>
Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Subtransaction commits and Hot Standby
Date: Tue, 16 Sep 2008 18:18:14 +0100
Message-ID: <1221585494.3913.1919.camel@ebony.2ndQuadrant> (raw)
In-Reply-To: <20080916141148.GA9229@alvh.no-ip.org>
References: <1221566023.3913.1823.camel@ebony.2ndQuadrant>
<48CFBC34.5010802@enterprisedb.com>
<20080916141148.GA9229@alvh.no-ip.org>
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
Attachments:atomic_subxids.v1.patch (text/x-patch, 11.1K) download
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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: simon@2ndQuadrant.com, alvherre@commandprompt.com, heikki.linnakangas@enterprisedb.com, pgsql-hackers@postgresql.org
Subject: Re: Subtransaction commits and Hot Standby
In-Reply-To: <1221585494.3913.1919.camel@ebony.2ndQuadrant>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox