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

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 1ca51ef69d610dae429b255dbde4dfc8334651ad..f1d3ca2f099d6c44498c1ae52d29879822e273f7 100644 (file)
--- 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
index d2e2e119970c1359b881fa02280cb95d7c1af416..79c9c0d844618f2a3c03eac48a714fbe2d00f021 100644 (file)
--- 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 ff08f869e2c62f9a5dc96d58c2dc77ec56ba7e95..980996e5eb83606f7d8da23b86175fa314119186 100644 (file)
--- 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 dd19fe9e386d12aaac375f36959d2d2f72cca907..942acb9eeb1536c55ce7289aa28c63e5f9366e1c 100644 (file)
--- 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 49ef8cc832bfd125af1b1a3d8660e295e9d7423f..ea030d695dc4352e8bc7d1142504196ebd59a072 100644 (file)
--- 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);