Optimize commit_siblings in two ways to improve group commit.

simonat2ndQuadrant · simonat2ndQuadrant · commit e620ee35b249 · 2010-12-08T18:48:03.000Z
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
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
@@ -1683,17 +1683,24 @@ SET ENABLE_SEQSCAN TO OFF;
       </indexterm>
       <listitem>
        <para>
-        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 <function>fsync()</function> 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.
+        <varname>commit_delay</varname> 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 <varname>wal_sync_method</varname>, 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
         <varname>commit_siblings</varname> 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.
        </para>
       </listitem>
      </varlistentry>
diff --git 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
@@ -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
@@ -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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -1816,7 +1816,7 @@ static struct config_int ConfigureNamesInt[] =`
`1816`	`1816`	`NULL`
`1817`	`1817`	`},`
`1818`	`1818`	`&CommitSiblings,`
`1819`		`- 5, 1, 1000, NULL, NULL`
	`1819`	`+ 5, 0, 1000, NULL, NULL`
`1820`	`1820`	`},`
`1821`	`1821`
`1822`	`1822`	`{`