From e620ee35b249b0af255ef788003d1c9edb815a35 Mon Sep 17 00:00:00 2001 From: Simon Riggs Date: Wed, 8 Dec 2010 18:48:03 +0000 Subject: [PATCH] Optimize commit_siblings in two ways to improve group commit. First, avoid scanning the whole ProcArray once we know there are at least commit_siblings active; second, skip the check altogether if commit_siblings = 0. Greg Smith --- doc/src/sgml/config.sgml | 17 ++++++++++++----- src/backend/access/transam/xact.c | 2 +- src/backend/storage/ipc/procarray.c | 17 ++++++++++++----- src/backend/utils/misc/guc.c | 2 +- src/include/storage/procarray.h | 2 +- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 1ca51ef69d..f1d3ca2f09 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1683,17 +1683,24 @@ SET ENABLE_SEQSCAN TO OFF; - Time delay between writing a commit record to the WAL buffer - and flushing the buffer out to disk, in microseconds. A - nonzero delay can allow multiple transactions to be committed - with only one fsync() system call, if + When the commit data for a transaction is flushed to disk, any + additional commits ready at that time are also flushed out. + commit_delay adds a time delay, set in + microseconds, before writing some commit records to the WAL + buffer and flushing the buffer out to disks. A nonzero delay + can allow more transactions to be committed with only one call + to the active wal_sync_method, if system load is high enough that additional transactions become ready to commit within the given interval. But the delay is just wasted if no other transactions become ready to commit. Therefore, the delay is only performed if at least commit_siblings other transactions are active at the instant that a server process has written its - commit record. The default is zero (no delay). + commit record. The default is zero (no delay). Since + all pending commit data flushes are written at every flush + regardless of this setting, it is rare that adding delay to + that by increasing this parameter will actually improve commit + performance. diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index d2e2e11997..79c9c0d844 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1052,7 +1052,7 @@ RecordTransactionCommit(void) * fewer than CommitSiblings other backends with active transactions. */ if (CommitDelay > 0 && enableFsync && - CountActiveBackends() >= CommitSiblings) + MinimumActiveBackends(CommitSiblings)) pg_usleep(CommitDelay); XLogFlush(XactLastRecEnd); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index ff08f869e2..980996e5eb 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -1886,20 +1886,25 @@ CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode) } /* - * CountActiveBackends --- count backends (other than myself) that are in - * active transactions. This is used as a heuristic to decide if + * MinimumActiveBackends --- count backends (other than myself) that are + * in active transactions. Return true if the count exceeds the + * minimum threshold passed. This is used as a heuristic to decide if * a pre-XLOG-flush delay is worthwhile during commit. * * Do not count backends that are blocked waiting for locks, since they are * not going to get to run until someone else commits. */ -int -CountActiveBackends(void) +bool +MinimumActiveBackends(int min) { ProcArrayStruct *arrayP = procArray; int count = 0; int index; + /* Quick short-circuit if no minimum is specified */ + if (min == 0) + return true; + /* * Note: for speed, we don't acquire ProcArrayLock. This is a little bit * bogus, but since we are only testing fields for zero or nonzero, it @@ -1932,9 +1937,11 @@ CountActiveBackends(void) if (proc->waitLock != NULL) continue; /* do not count if blocked on a lock */ count++; + if (count >= min) + break; } - return count; + return count >= min; } /* diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index dd19fe9e38..942acb9eeb 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1816,7 +1816,7 @@ static struct config_int ConfigureNamesInt[] = NULL }, &CommitSiblings, - 5, 1, 1000, NULL, NULL + 5, 0, 1000, NULL, NULL }, { diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h index 49ef8cc832..ea030d695d 100644 --- a/src/include/storage/procarray.h +++ b/src/include/storage/procarray.h @@ -60,7 +60,7 @@ extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin, extern VirtualTransactionId *GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid); extern pid_t CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode); -extern int CountActiveBackends(void); +extern bool MinimumActiveBackends(int min); extern int CountDBBackends(Oid databaseid); extern void CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conflictPending); extern int CountUserBackends(Oid roleid);