[HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds

Hi hackers,

I have a workload using SSI that causes a lot of tuple predicate to be
promoted to page locks.  This causes a lot of spurious serialisability
errors, since the promotion happens at once three tuples on a page are
locked, and the affected tables have 30-90 tuples per page.

PredicateLockPromotionThreshold() has the following comment:

 * TODO SSI: We should do something more intelligent about what the
 * thresholds are, either making it proportional to the number of
 * tuples in a page & pages in a relation, or at least making it a
 * GUC. 

Attached is a patch that does the "at least" part of this.

One thing I don't like about this patch is that if a user has increased
max_pred_locks_per_transaction, they need to set
max_pred_locks_per_relation to half of that to retain the current
behaviour, or they'll suddenly find themselves with a lot more relation
locks.  If it's possible to make a GUCs default value dependent on the
value of another, that could be a solution.  Otherwise, the page lock
threshold GUC could be changed to be expressed as a fraction of
max_pred_locks_per_transaction, to keep the current behaviour.


Cheers,

Ilmari

From bb81a54ee6c9a4855f6aeb52b968d188f44b14ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 12 Dec 2016 17:57:33 +0000
Subject: [PATCH] Add GUCs for predicate lock promotion thresholds

This addresses part of the TODO comment for predicate lock promotion
threshold, by making them configurable.  The default values are the same
as what used to be hardcoded.
---
 doc/src/sgml/config.sgml                      | 36 +++++++++++++++++++++++++++
 doc/src/sgml/mvcc.sgml                        |  4 ++-
 src/backend/storage/lmgr/predicate.c          | 18 +++++++++-----
 src/backend/utils/misc/guc.c                  | 22 ++++++++++++++++
 src/backend/utils/misc/postgresql.conf.sample |  3 +++
 src/include/storage/predicate.h               |  2 ++
 6 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0fc4e57d90..6e133ffebd 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7172,6 +7172,42 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-max-pred-locks-per-relation" xreflabel="max_pred_locks_per_relation">
+      <term><varname>max_pred_locks_per_relation</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>max_pred_locks_per_relation</> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        This controls how many pages of a single relation can be
+        predicate-locked before the lock is promoted to covering the whole
+        relation.  The default is 32.  In previous versions
+        of <productname>PostgreSQL</> it used to be hard-coded to half
+        of <xref linkend="guc-max-pred-locks-per-transaction">, and you might
+        want to raise this value if you raise that.  This parameter can only
+        be set at server start.
+       </para>
+
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-max-pred-locks-per-page" xreflabel="max_pred_locks_per_page">
+      <term><varname>max_pred_locks_per_page</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>max_pred_locks_per_page</> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        This controls how many rows on a single page can be predicate-locked
+        before the lock is promoted to covering the whole page.  The default
+        is 3.  This parameter can only be set at server start.
+       </para>
+
+      </listitem>
+     </varlistentry>
+
      </variablelist>
    </sect1>
 
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index 306def4a15..4652cdf094 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -765,7 +765,9 @@ ERROR:  could not serialize access due to read/write dependencies among transact
        locks into a single relation-level predicate lock because the predicate
        lock table is short of memory, an increase in the rate of serialization
        failures may occur.  You can avoid this by increasing
-       <xref linkend="guc-max-pred-locks-per-transaction">.
+       <xref linkend="guc-max-pred-locks-per-transaction">,
+       <xref linkend="guc-max-pred-locks-per-relation"> and/or
+       <xref linkend="guc-max-pred-locks-per-page">.
       </para>
      </listitem>
      <listitem>
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 24ed21b487..e73b2b417c 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -355,6 +355,12 @@ static SERIALIZABLEXACT *OldCommittedSxact;
 /* This configuration variable is used to set the predicate lock table size */
 int            max_predicate_locks_per_xact;        /* set by guc.c */
 
+/* This configuration variable is used to decide when to upgrade a page lock to a relation lock */
+int            max_predicate_locks_per_relation;    /* set by guc.c */
+
+/* This configuration variable is used to decide when to upgrade a row lock to a page lock */
+int            max_predicate_locks_per_page;        /* set by guc.c */
+
 /*
  * This provides a list of objects in order to track transactions
  * participating in predicate locking.  Entries in the list are fixed size,
@@ -2124,10 +2130,10 @@ DeleteChildTargetLocks(const PREDICATELOCKTARGETTAG *newtargettag)
  * descendants, e.g. both tuples and pages for a relation lock.
  *
  * TODO SSI: We should do something more intelligent about what the
- * thresholds are, either making it proportional to the number of
- * tuples in a page & pages in a relation, or at least making it a
- * GUC. Currently the threshold is 3 for a page lock, and
- * max_pred_locks_per_transaction/2 for a relation lock, chosen
+ * thresholds are, e.g. making it proportional to the number of tuples
+ * in a page & pages in a relation. Currently the default threshold is
+ * 3 for a page lock, and 32 (half of the default value for
+ * max_pred_locks_per_transaction) for a relation lock, chosen
  * entirely arbitrarily (and without benchmarking).
  */
 static int
@@ -2136,10 +2142,10 @@ PredicateLockPromotionThreshold(const PREDICATELOCKTARGETTAG *tag)
     switch (GET_PREDICATELOCKTARGETTAG_TYPE(*tag))
     {
         case PREDLOCKTAG_RELATION:
-            return max_predicate_locks_per_xact / 2;
+            return max_predicate_locks_per_relation;
 
         case PREDLOCKTAG_PAGE:
-            return 3;
+            return max_predicate_locks_per_page;
 
         case PREDLOCKTAG_TUPLE:
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a02511754e..cdb6b06181 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2194,6 +2194,28 @@ static struct config_int ConfigureNamesInt[] =
         NULL, NULL, NULL
     },
 
+    {
+        {"max_pred_locks_per_relation", PGC_POSTMASTER, LOCK_MANAGEMENT,
+            gettext_noop("Sets the maximum number of predicate-locked pages per relation."),
+            gettext_noop("If more than this number of pages in the same relation are locked "
+                         "the lock is promoted to a relation level lock.")
+        },
+        &max_predicate_locks_per_relation,
+        32, 5, INT_MAX,
+        NULL, NULL, NULL
+    },
+
+    {
+        {"max_pred_locks_per_page", PGC_POSTMASTER, LOCK_MANAGEMENT,
+            gettext_noop("Sets the maximum number of predicate-locked rows per page."),
+            gettext_noop("If more than this number of rows on the same page are locked "
+                         "the lock is promoted to a page level lock.")
+        },
+        &max_predicate_locks_per_page,
+        3, 1, INT_MAX,
+        NULL, NULL, NULL
+    },
+
     {
         {"authentication_timeout", PGC_SIGHUP, CONN_AUTH_SECURITY,
             gettext_noop("Sets the maximum allowed time to complete client authentication."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 7f9acfda06..c546795972 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -590,6 +590,9 @@
                     # (change requires restart)
 #max_pred_locks_per_transaction = 64    # min 10
                     # (change requires restart)
+#max_pred_locks_per_relation = 32    # min 5
+                    # (change requires restart)
+#max_pred_locks_per_page = 3        # (change requires restart)
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h
index a66b5b7134..7774a9417f 100644
--- a/src/include/storage/predicate.h
+++ b/src/include/storage/predicate.h
@@ -22,6 +22,8 @@
  * GUC variables
  */
 extern int    max_predicate_locks_per_xact;
+extern int    max_predicate_locks_per_relation;
+extern int    max_predicate_locks_per_page;
 
 
 /* Number of SLRU buffers to use for predicate locking */
-- 
2.11.0


-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers