From: Marc G. Fournier Date: Fri, 28 Mar 1997 07:06:53 +0000 (+0000) Subject: From: Dan McGuirk X-Git-Tag: REL9_0_0~29102 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=159f8c63ad34db0582e375b4e04916128be2a0e4;p=pg-rex%2Fsyncrep.git From: Dan McGuirk Reply-To: hackers@hub.org, Dan McGuirk To: hackers@hub.org Subject: [HACKERS] tmin writeback optimization I was doing some profiling of the backend, and noticed that during a certain benchmark I was running somewhere between 30% and 75% of the backend's CPU time was being spent in calls to TransactionIdDidCommit() from HeapTupleSatisfiesNow() or HeapTupleSatisfiesItself() to determine that changed rows' transactions had in fact been committed even though the rows' tmin values had not yet been set. When a query looks at a given row, it needs to figure out whether the transaction that changed the row has been committed and hence it should pay attention to the row, or whether on the other hand the transaction is still in progress or has been aborted and hence the row should be ignored. If a tmin value is set, it is known definitively that the row's transaction has been committed. However, if tmin is not set, the transaction referred to in xmin must be looked up in pg_log, and this is what the backend was spending a lot of time doing during my benchmark. So, implementing a method suggested by Vadim, I created the following patch that, the first time a query finds a committed row whose tmin value is not set, sets it, and marks the buffer where the row is stored as dirty. (It works for tmax, too.) This doesn't result in the boost in real time performance I was hoping for, however it does decrease backend CPU usage by up to two-thirds in certain situations, so it could be rather beneficial in high-concurrency settings. --- diff --git a/src/backend/access/common/heapvalid.c b/src/backend/access/common/heapvalid.c index fdf333092f..f0b4f3f345 100644 --- a/src/backend/access/common/heapvalid.c +++ b/src/backend/access/common/heapvalid.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/common/Attic/heapvalid.c,v 1.12 1996/11/10 02:56:47 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/common/Attic/heapvalid.c,v 1.13 1997/03/28 07:03:53 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -21,6 +21,7 @@ #include #include #include +#include /* ---------------- * heap_keytest @@ -89,14 +90,16 @@ heap_keytest(HeapTuple t, HeapTuple heap_tuple_satisfies(ItemId itemId, Relation relation, + Buffer buffer, PageHeader disk_page, TimeQual qual, int nKeys, ScanKey key) { - HeapTuple tuple; + HeapTuple tuple, result; bool res; - + TransactionId old_tmin, old_tmax; + if (! ItemIdIsUsed(itemId)) return NULL; @@ -107,12 +110,26 @@ heap_tuple_satisfies(ItemId itemId, nKeys, key); else res = TRUE; - - if (res && (relation->rd_rel->relkind == RELKIND_UNCATALOGED - || HeapTupleSatisfiesTimeQual(tuple,qual))) - return tuple; - - return (HeapTuple) NULL; + + result = (HeapTuple)NULL; + if (res) { + if(relation->rd_rel->relkind == RELKIND_UNCATALOGED) { + result = tuple; + } else { + old_tmin = tuple->t_tmin; + old_tmax = tuple->t_tmax; + res = HeapTupleSatisfiesTimeQual(tuple,qual); + if(tuple->t_tmin != old_tmin || + tuple->t_tmax != old_tmax) { + SetBufferCommitInfoNeedsSave(buffer); + } + if(res) { + result = tuple; + } + } + } + + return result; } /* diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index d559a3da6d..67806925b1 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.10 1997/01/23 05:59:47 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.11 1997/03/28 07:04:11 scrappy Exp $ * * * INTERFACE ROUTINES @@ -388,7 +388,7 @@ heapgettup(Relation relation, * if current tuple qualifies, return it. * ---------------- */ - if ((rtup = heap_tuple_satisfies(lpp, relation, (PageHeader) dp, + if ((rtup = heap_tuple_satisfies(lpp, relation, *b, (PageHeader) dp, timeQual, nkeys, key)) != NULL) { ItemPointer iptr = &(rtup->t_ctid); if (ItemPointerGetBlockNumber(iptr) != page) { @@ -1009,7 +1009,7 @@ heap_fetch(Relation relation, * ---------------- */ - tuple = heap_tuple_satisfies(lp, relation, dp, + tuple = heap_tuple_satisfies(lp, relation, buffer, dp, timeQual, 0,(ScanKey)NULL); if (tuple == NULL) @@ -1154,7 +1154,7 @@ heap_delete(Relation relation, ItemPointer tid) * check that we're deleteing a valid item * ---------------- */ - if (!(tp = heap_tuple_satisfies(lp, relation, dp, + if (!(tp = heap_tuple_satisfies(lp, relation, b, dp, NowTimeQual, 0, (ScanKey) NULL))) { /* XXX call something else */ @@ -1282,6 +1282,7 @@ heap_replace(Relation relation, ItemPointer otid, HeapTuple tup) */ if (!heap_tuple_satisfies(lp, relation, + buffer, (PageHeader)dp, NowTimeQual, 0, diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c index 7613971df7..dcfebd0d43 100644 --- a/src/backend/storage/buffer/buf_init.c +++ b/src/backend/storage/buffer/buf_init.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/buffer/buf_init.c,v 1.7 1997/01/26 00:45:25 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/buffer/buf_init.c,v 1.8 1997/03/28 07:04:52 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -64,6 +64,8 @@ extern IpcSemaphoreId WaitIOSemId; long *PrivateRefCount; /* also used in freelist.c */ long *LastRefCount; /* refcounts of last ExecMain level */ +long *CommitInfoNeedsSave; /* to write buffers where we have filled in */ + /* t_tmin (or t_tmax) */ /* * Data Structures: @@ -236,6 +238,7 @@ InitBufferPool(IPCKey key) #endif PrivateRefCount = (long *) calloc(NBuffers, sizeof(long)); LastRefCount = (long *) calloc(NBuffers, sizeof(long)); + CommitInfoNeedsSave = (long *) calloc(NBuffers, sizeof(long)); } /* ----------------------------------------------------- diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index b40b0b0e04..5331ab3fbe 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.10 1997/01/23 19:43:23 scrappy Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.11 1997/03/28 07:05:03 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -848,6 +848,10 @@ ReleaseAndReadBuffer(Buffer buffer, AddBufferToFreelist(bufHdr); bufHdr->flags |= BM_FREE; } + if(CommitInfoNeedsSave[buffer - 1]) { + bufHdr->flags |= (BM_DIRTY | BM_JUST_DIRTIED); + CommitInfoNeedsSave[buffer - 1] = 0; + } retbuf = ReadBufferWithBufferLock(relation, blockNum, true); return retbuf; } @@ -1083,6 +1087,7 @@ ResetBufferPool() { register int i; for (i=1; i<=NBuffers; i++) { + CommitInfoNeedsSave[i - 1] = 0; if (BufferIsValid(i)) { while(PrivateRefCount[i - 1] > 0) { ReleaseBuffer(i); @@ -1498,6 +1503,10 @@ ReleaseBuffer(Buffer buffer) AddBufferToFreelist(bufHdr); bufHdr->flags |= BM_FREE; } + if(CommitInfoNeedsSave[buffer - 1]) { + bufHdr->flags |= (BM_DIRTY | BM_JUST_DIRTIED); + CommitInfoNeedsSave[buffer - 1] = 0; + } SpinRelease(BufMgrLock); } @@ -1734,3 +1743,8 @@ int SetBufferWriteMode (int mode) WriteMode = mode; return (old); } + +void SetBufferCommitInfoNeedsSave(Buffer buffer) +{ + CommitInfoNeedsSave[buffer - 1]++; +} diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index dd115b6988..5c2f8271fa 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/time/tqual.c,v 1.1.1.1 1996/07/09 06:22:10 scrappy Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/time/tqual.c,v 1.2 1997/03/28 07:05:28 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -478,6 +478,8 @@ HeapTupleSatisfiesTimeQual(HeapTuple tuple, TimeQual qual) /* * HeapTupleSatisfiesItself -- * True iff heap tuple is valid for "itself." + * "{it}self" means valid as of everything that's happened + * in the current transaction, _including_ the current command. * * Note: * Assumes heap tuple is valid. @@ -485,11 +487,15 @@ HeapTupleSatisfiesTimeQual(HeapTuple tuple, TimeQual qual) /* * The satisfaction of "itself" requires the following: * - * ((Xmin == my-transaction && (Xmax is null [|| Xmax != my-transaction)]) + * ((Xmin == my-transaction && the row was updated by the current transaction, and + * (Xmax is null it was not deleted + * [|| Xmax != my-transaction)]) [or it was deleted by another transaction] * || * - * (Xmin is committed && - * (Xmax is null || (Xmax != my-transaction && Xmax is not committed))) + * (Xmin is committed && the row was modified by a committed transaction, and + * (Xmax is null || the row has not been deleted, or + * (Xmax != my-transaction && the row was deleted by another transaction + * Xmax is not committed))) that has not been committed */ static bool HeapTupleSatisfiesItself(HeapTuple tuple) @@ -508,6 +514,8 @@ HeapTupleSatisfiesItself(HeapTuple tuple) if (!TransactionIdDidCommit((TransactionId)tuple->t_xmin)) { return (false); } + + tuple->t_tmin = TransactionIdGetCommitTime(tuple->t_xmin); } /* the tuple was inserted validly */ @@ -522,13 +530,23 @@ HeapTupleSatisfiesItself(HeapTuple tuple) if (TransactionIdIsCurrentTransactionId((TransactionId)tuple->t_xmax)) { return (false); } + + if (!TransactionIdDidCommit((TransactionId)tuple->t_xmax)) { + return (true); + } - return ((bool)!TransactionIdDidCommit((TransactionId)tuple->t_xmax)); + /* by here, deleting transaction has committed */ + tuple->t_tmax = TransactionIdGetCommitTime(tuple->t_xmax); + + return (false); } /* * HeapTupleSatisfiesNow -- * True iff heap tuple is valid "now." + * "now" means valid including everything that's happened + * in the current transaction _up to, but not including,_ + * the current command. * * Note: * Assumes heap tuple is valid. @@ -536,13 +554,19 @@ HeapTupleSatisfiesItself(HeapTuple tuple) /* * The satisfaction of "now" requires the following: * - * ((Xmin == my-transaction && Cmin != my-command && - * (Xmax is null || (Xmax == my-transaction && Cmax != my-command))) - * || + * ((Xmin == my-transaction && changed by the current transaction + * Cmin != my-command && but not by this command, and + * (Xmax is null || the row has not been deleted, or + * (Xmax == my-transaction && it was deleted by the current transaction + * Cmax != my-command))) but not by this command, + * || or * - * (Xmin is committed && - * (Xmax is null || (Xmax == my-transaction && Cmax == my-command) || - * (Xmax is not committed && Xmax != my-transaction)))) + * (Xmin is committed && the row was modified by a committed transaction, and + * (Xmax is null || the row has not been deleted, or + * (Xmax == my-transaction && the row is being deleted by this command, or + * Cmax == my-command) || + * (Xmax is not committed && the row was deleted by another transaction + * Xmax != my-transaction)))) that has not been committed * * mao says 17 march 1993: the tests in this routine are correct; * if you think they're not, you're wrong, and you should think @@ -605,6 +629,13 @@ HeapTupleSatisfiesNow(HeapTuple tuple) if (!TransactionIdDidCommit((TransactionId)tuple->t_xmin)) { return (false); } + + /* + * the transaction has been committed--store the commit time _now_ + * instead of waiting for a vacuum so we avoid the expensive call + * next time. + */ + tuple->t_tmin = TransactionIdGetCommitTime(tuple->t_xmin); } /* by here, the inserting transaction has committed */ @@ -615,12 +646,18 @@ HeapTupleSatisfiesNow(HeapTuple tuple) if (TransactionIdIsCurrentTransactionId((TransactionId)tuple->t_xmax)) { return (false); } - + + if (AbsoluteTimeIsBackwardCompatiblyReal(tuple->t_tmax)) { + return (false); + } + if (!TransactionIdDidCommit((TransactionId)tuple->t_xmax)) { - return (true); + return (true); } - - /* by here, deleting transaction has committed */ + + /* xmax transaction committed, but no tmax set. so set it. */ + tuple->t_tmax = TransactionIdGetCommitTime(tuple->t_xmax); + return (false); } diff --git a/src/include/access/valid.h b/src/include/access/valid.h index 4ab525dec9..ca209a16e5 100644 --- a/src/include/access/valid.h +++ b/src/include/access/valid.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: valid.h,v 1.3 1996/11/05 10:37:07 scrappy Exp $ + * $Id: valid.h,v 1.4 1997/03/28 07:05:54 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -26,7 +26,9 @@ extern bool heap_keytest(HeapTuple t, TupleDesc tupdesc, int nkeys, ScanKey keys); extern HeapTuple heap_tuple_satisfies(ItemId itemId, Relation relation, - PageHeader disk_page, TimeQual qual, int nKeys, ScanKey key); + Buffer buffer, PageHeader disk_page, + TimeQual qual, int nKeys, + ScanKey key); extern bool TupleUpdatedByCurXactAndCmd(HeapTuple t); diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 356d2d8e2d..4fada24503 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: buf_internals.h,v 1.11 1997/01/25 03:09:33 momjian Exp $ + * $Id: buf_internals.h,v 1.12 1997/03/28 07:06:48 scrappy Exp $ * * NOTE * If BUFFERPAGE0 is defined, then 0 will be used as a @@ -216,6 +216,7 @@ extern BufferDesc *BufferDescriptors; extern BufferBlock BufferBlocks; extern long *PrivateRefCount; extern long *LastRefCount; +extern long *CommitInfoNeedsSave; extern SPINLOCK BufMgrLock; /* localbuf.c */ diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index e4e376e000..fc239caeed 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: bufmgr.h,v 1.9 1997/01/16 07:53:27 vadim Exp $ + * $Id: bufmgr.h,v 1.10 1997/03/28 07:06:53 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -114,6 +114,7 @@ extern int ReleaseAndReadBuffer_Debug(char *file, extern void BufferRefCountReset(int *refcountsave); extern void BufferRefCountRestore(int *refcountsave); extern int SetBufferWriteMode (int mode); +extern void SetBufferCommitInfoNeedsSave(Buffer buffer); #endif /* !defined(BufMgrIncluded) */