Обсуждение: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
[HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
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
On Wed, Dec 14, 2016 at 8:16 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote: > Attached is a patch Please add this to the open CommitFest to ensure that it is reviewed. http://commitfest.postgresql.org/action/commitfest_view/open -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Kevin Grittner <kgrittn@gmail.com> writes: > On Wed, Dec 14, 2016 at 8:16 AM, Dagfinn Ilmari Mannsåker > <ilmari@ilmari.org> wrote: > >> Attached is a patch > > Please add this to the open CommitFest to ensure that it is reviewed. Done. https://commitfest.postgresql.org/12/910/ -- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live withthe consequences of." --Skud's Meta-Law
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > 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. Another option would be to have a special sentinel (e.g. -1) which is the default, and keeps the current behaviour. -- "A disappointingly low fraction of the human race is,at any given time, on fire." - Stig Sandbeck Mathisen
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
> ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
>> 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.
> Another option would be to have a special sentinel (e.g. -1) which is
> the default, and keeps the current behaviour.
FWIW, interdependent GUC defaults are generally unworkable.
See commit a16d421ca and the sorry history that led up to it.
I think you could make it work as a fraction, though.
regards, tom lane
On Thu, Jan 5, 2017 at 12:19 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: >> ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: >>> 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. > >> Another option would be to have a special sentinel (e.g. -1) which is >> the default, and keeps the current behaviour. > > FWIW, interdependent GUC defaults are generally unworkable. > See commit a16d421ca and the sorry history that led up to it. > I think you could make it work as a fraction, though. I think that, ultimately, both an fraction of actual (the number of tuples on a page or the number of pages in a relation) *and* an absolute number (as this patch implements) could be useful. The former would be more "adaptable" -- providing reasonable behavior for different sized tuples and different sized tables, while the latter would prevent a single table with very small tuples or a lot of pages from starving all other uses. This patch implements the easier part, and is likely to be very useful to many users without the other part, so I think it is worth accepting as a step in the right direction, and consistent with not letting the good be the enemy of the perfect. There are a couple minor formatting issues I can clean up as committer, but there are a couple more substantive things to note. (1) The new GUCs are named max_pred_locks_per_*, but the implementation passes them unmodified from a function specifying at what count the lock should be promoted. We either need to change the names or (probably better, only because I can't think of a good name for the other way) return the GUC value + 1 from the function. Or maybe change the function name and how the returned number is used, if we care about eliminating the overhead of the increment instruction. That actually seems least confusing. (2) The new GUCs are declared and documented to be set only on startup. This was probably just copied from max_pred_locks_per_transaction, but that sets an allocation size while these don't. I think these new GUCs should be PGC_SIGHUP, and documented to change on reload. Other than that, I think it is in good shape and I am would feel good about committing it. Of course, if there are any objections to it, I will not go ahead without reaching consensus. I am on vacation next week, so I would not do so before then; in fact I may not have a chance to respond to any comments before then. If the author wishes to make these changes, great; otherwise I can do it before commit. I will mark this Ready for Committer. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
A couple things occurred to me after hitting "Send". In addition to the prior 2 points: (3) The documentation for max_pred_locks_per_relation needs a fix. Both page locks and tuple locks for the relation are counted toward the limit. In releases prior to this patch, max_pred_locks_per_relation was calculated as "max_pred_locks_per_transaction / 2". I know that people have sometimes increased max_pred_locks_per_transaction specifically to raise the limit on locks within a relation before the promotion to relation granularity occurs. It seems kinda anti-social not to support a special value for continuing that behavior or, if we don't want to do that, at least modifying pg_upgrade to set max_pred_locks_per_relation to the value that was in effect in the old version. In any event, it seems more like autovacuum_work_mem or autovacuum_vacuum_cost_limit than like effective_cache_size. Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Kevin Grittner <kgrittn@gmail.com> writes:
> (1) The new GUCs are named max_pred_locks_per_*, but the
> implementation passes them unmodified from a function specifying at
> what count the lock should be promoted. We either need to change
> the names or (probably better, only because I can't think of a good
> name for the other way) return the GUC value + 1 from the function.
> Or maybe change the function name and how the returned number is
> used, if we care about eliminating the overhead of the increment
> instruction. That actually seems least confusing.
I've done the latter, and renamed the function MaxPredicateChildLocks.
I also changed the default for max_pred_locks_per_page from 3 to 4 and
the minimum to 0, to keep the effective thresholds the same.
> (2) The new GUCs are declared and documented to be set only on
> startup. This was probably just copied from
> max_pred_locks_per_transaction, but that sets an allocation size
> while these don't. I think these new GUCs should be PGC_SIGHUP,
> and documented to change on reload.
Fixed.
> (3) The documentation for max_pred_locks_per_relation needs a fix.
> Both page locks and tuple locks for the relation are counted toward
> the limit.
Fixed.
> In releases prior to this patch, max_pred_locks_per_relation was
> calculated as "max_pred_locks_per_transaction / 2". I know that
> people have sometimes increased max_pred_locks_per_transaction
> specifically to raise the limit on locks within a relation before
> the promotion to relation granularity occurs. It seems kinda
> anti-social not to support a special value for continuing that
> behavior or, if we don't want to do that, at least modifying
> pg_upgrade to set max_pred_locks_per_relation to the value that was
> in effect in the old version.
This is exactly what we've been doing at my workplace, and I mentioned
this in my original email:
>> ilmari@ilmari.org (Dagfinn Ilmari Manns�ker) writes:
>>> 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.
>
>> Another option would be to have a special sentinel (e.g. -1) which is
>> the default, and keeps the current behaviour.
The attached updated patch combines the two behaviours. Positive values
mean an absolute number of locks, while negative values mean
max_predicate_locks_per_transaction / -max_predicate_locks_per_relation.
Making the default value -2 keeps backwards compatibility.
Alternatively we could have a separate setting for the fraction (which
could then be a floating-point number, for finer control), and use the
absolute value if that is zero.
Regards,
Ilmari
--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen
From 12c55660e9235e100b8802645eb8aaef7683888f 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 | 39 +++++++++++++++++++++++++++
doc/src/sgml/mvcc.sgml | 4 ++-
src/backend/storage/lmgr/predicate.c | 37 +++++++++++++++----------
src/backend/utils/misc/guc.c | 22 +++++++++++++++
src/backend/utils/misc/postgresql.conf.sample | 2 ++
src/include/storage/predicate.h | 2 ++
6 files changed, 91 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index fb5d6473ef..1f944aab1c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7255,6 +7255,45 @@ 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 or tuples of a single relation can be
+ predicate-locked before the lock is promoted to covering the whole
+ relation. Values greater than or equal to zero mean an absolute
+ limit, while negative values
+ mean <xref linkend="guc-max-pred-locks-per-transaction"> divided by
+ the absolute value of this setting. The default is -2, which keeps
+ the behaviour from previous versions of <productname>PostgreSQL</>.
+ This parameter can only be set in the <filename>postgresql.conf</>
+ file or on the server command line.
+ </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 4. This parameter can only be set in
+ the <filename>postgresql.conf</> file or on the server command line.
+ </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 9183764ca7..ff50e5f9fb 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,
@@ -437,7 +443,7 @@ static void RestoreScratchTarget(bool lockheld);
static void RemoveTargetIfNoLongerUsed(PREDICATELOCKTARGET *target,
uint32 targettaghash);
static void DeleteChildTargetLocks(const PREDICATELOCKTARGETTAG *newtargettag);
-static int PredicateLockPromotionThreshold(const PREDICATELOCKTARGETTAG *tag);
+static int MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag);
static bool CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag);
static void DecrementParentLocks(const PREDICATELOCKTARGETTAG *targettag);
static void CreatePredicateLock(const PREDICATELOCKTARGETTAG *targettag,
@@ -2118,28 +2124,31 @@ DeleteChildTargetLocks(const PREDICATELOCKTARGETTAG *newtargettag)
}
/*
- * Returns the promotion threshold for a given predicate lock
- * target. This is the number of descendant locks required to promote
- * to the specified tag. Note that the threshold includes non-direct
- * descendants, e.g. both tuples and pages for a relation lock.
+ * Returns the promotion limit for a given predicate lock target.
+ * This is the max number of descendant locks allowed before
+ * promoting to the specified tag. Note that the limit includes
+ * non-direct 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
+ * limits are, e.g. making it proportional to the number of tuples
+ * in a page & pages in a relation. Currently the default limit is
+ * 4 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
-PredicateLockPromotionThreshold(const PREDICATELOCKTARGETTAG *tag)
+MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag)
{
switch (GET_PREDICATELOCKTARGETTAG_TYPE(*tag))
{
case PREDLOCKTAG_RELATION:
- return max_predicate_locks_per_xact / 2;
+ return max_predicate_locks_per_relation < 0
+ ? max_predicate_locks_per_xact / -max_predicate_locks_per_relation
+ : max_predicate_locks_per_relation;
case PREDLOCKTAG_PAGE:
- return 3;
+ return max_predicate_locks_per_page;
case PREDLOCKTAG_TUPLE:
@@ -2194,8 +2203,8 @@ CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag)
else
parentlock->childLocks++;
- if (parentlock->childLocks >=
- PredicateLockPromotionThreshold(&targettag))
+ if (parentlock->childLocks >
+ MaxPredicateChildLocks(&targettag))
{
/*
* We should promote to this parent lock. Continue to check its
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5f43b1ec92..af7929ae7f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2186,6 +2186,28 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"max_pred_locks_per_relation", PGC_SIGHUP, LOCK_MANAGEMENT,
+ gettext_noop("Sets the maximum number of predicate-locked pages per relation."),
+ gettext_noop("If more than this number of pages or tuples in the same relation are locked "
+ "the lock is promoted to a relation level lock.")
+ },
+ &max_predicate_locks_per_relation,
+ -2, INT_MIN, INT_MAX,
+ NULL, NULL, NULL
+ },
+
+ {
+ {"max_pred_locks_per_page", PGC_SIGHUP, 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,
+ 4, 0, 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 661b0fa9b6..fe2c7096dd 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -591,6 +591,8 @@
# (change requires restart)
#max_pred_locks_per_transaction = 64 # min 10
# (change requires restart)
+#max_pred_locks_per_relation = -2 # negative values mean max_pred_locks_per_transaction / -N
+#max_pred_locks_per_page = 3 # min 0
#------------------------------------------------------------------------------
diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h
index f996d8e818..8f9ea29917 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jan 23, 2017 at 9:50 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote: > Kevin Grittner <kgrittn@gmail.com> writes: > >> (1) The new GUCs are named max_pred_locks_per_*, but the >> implementation passes them unmodified from a function specifying at >> what count the lock should be promoted. We either need to change >> the names or (probably better, only because I can't think of a good >> name for the other way) return the GUC value + 1 from the function. >> Or maybe change the function name and how the returned number is >> used, if we care about eliminating the overhead of the increment >> instruction. That actually seems least confusing. > > I've done the latter, and renamed the function MaxPredicateChildLocks. > I also changed the default for max_pred_locks_per_page from 3 to 4 and > the minimum to 0, to keep the effective thresholds the same. > >> (2) The new GUCs are declared and documented to be set only on >> startup. This was probably just copied from >> max_pred_locks_per_transaction, but that sets an allocation size >> while these don't. I think these new GUCs should be PGC_SIGHUP, >> and documented to change on reload. > > Fixed. > >> (3) The documentation for max_pred_locks_per_relation needs a fix. >> Both page locks and tuple locks for the relation are counted toward >> the limit. > > Fixed. > >> In releases prior to this patch, max_pred_locks_per_relation was >> calculated as "max_pred_locks_per_transaction / 2". I know that >> people have sometimes increased max_pred_locks_per_transaction >> specifically to raise the limit on locks within a relation before >> the promotion to relation granularity occurs. It seems kinda >> anti-social not to support a special value for continuing that >> behavior or, if we don't want to do that, at least modifying >> pg_upgrade to set max_pred_locks_per_relation to the value that was >> in effect in the old version. > > This is exactly what we've been doing at my workplace, and I mentioned > this in my original email: > >>> ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: >>>> 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. >> >>> Another option would be to have a special sentinel (e.g. -1) which is >>> the default, and keeps the current behaviour. > > The attached updated patch combines the two behaviours. Positive values > mean an absolute number of locks, while negative values mean > max_predicate_locks_per_transaction / -max_predicate_locks_per_relation. > Making the default value -2 keeps backwards compatibility. > > Alternatively we could have a separate setting for the fraction (which > could then be a floating-point number, for finer control), and use the > absolute value if that is zero. > > Regards, > > Ilmari > > -- > "A disappointingly low fraction of the human race is, > at any given time, on fire." - Stig Sandbeck Mathisen > -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Jan 23, 2017 at 9:50 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote: > [new patch addressing issues raised] Thanks! >> In releases prior to this patch, max_pred_locks_per_relation was >> calculated as "max_pred_locks_per_transaction / 2". I know that >> people have sometimes increased max_pred_locks_per_transaction >> specifically to raise the limit on locks within a relation before >> the promotion to relation granularity occurs. It seems kinda >> anti-social not to support a special value for continuing that >> behavior or, if we don't want to do that, at least modifying >> pg_upgrade to set max_pred_locks_per_relation to the value that was >> in effect in the old version. > > This is exactly what we've been doing at my workplace It occurred to me that it would make sense to allow these settings to be attached to a database or table (though *not* a role). I'll look at that when I get back if you don't get to it first. >>> ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: >>>> 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. >> >>> Another option would be to have a special sentinel (e.g. -1) which is >>> the default, and keeps the current behaviour. > > The attached updated patch combines the two behaviours. Positive values > mean an absolute number of locks, while negative values mean > max_predicate_locks_per_transaction / -max_predicate_locks_per_relation. > Making the default value -2 keeps backwards compatibility. > > Alternatively we could have a separate setting for the fraction (which > could then be a floating-point number, for finer control), and use the > absolute value if that is zero. I will need some time to consider that.... -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Jan 24, 2017 at 3:32 AM, Kevin Grittner <kgrittn@gmail.com> wrote: > I will need some time to consider that.... Moved to CF 2017-03 for now. The last patch sent still applies. -- Michael
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Kevin Grittner <kgrittn@gmail.com> writes:
> It occurred to me that it would make sense to allow these settings
> to be attached to a database or table (though *not* a role). I'll
> look at that when I get back if you don't get to it first.
Attached is a draft patch (no docs or tests) on top of the previous one,
adding reloptions for the per-relation and per-page thresholds. That
made me realise that the corresponding GUCs don't need to be PGC_SIGHUP,
but could be PGC_SUSET or PGC_USERSET. I'll adjust the original patch
if you agree. I have not got around around to adding per-database
versions of the setting, but could have a stab at that.
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl
From e2ae108ca8212790604ef7e54f278e41ca460ffb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 25 Jan 2017 00:13:53 +0000
Subject: [PATCH 2/2] Add max_pred_locks_per_{relation,page} reloptions
---
src/backend/access/common/reloptions.c | 24 +++++++++++++++++++++-
src/backend/storage/lmgr/predicate.c | 37 +++++++++++++++++++++++-----------
src/bin/psql/tab-complete.c | 2 ++
src/include/utils/rel.h | 21 +++++++++++++++++++
4 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 42b4ea410f..ab8977a218 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -284,6 +284,24 @@ static relopt_int intRelOpts[] =
},
-1, 0, 1024
},
+ {
+ {
+ "max_pred_locks_per_relation",
+ "Maximum number of pages or rows that can be predicate-locked before locking the whole relation.",
+ RELOPT_KIND_HEAP,
+ AccessExclusiveLock,
+ },
+ -1, 0, INT_MAX
+ },
+ {
+ {
+ "max_pred_locks_per_page",
+ "Maximum number of rows on a single page that can be predicate-locked before locking the whole page.",
+ RELOPT_KIND_HEAP,
+ AccessExclusiveLock,
+ },
+ -1, 0, INT_MAX
+ },
/* list terminator */
{{NULL}}
@@ -1310,7 +1328,11 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{"user_catalog_table", RELOPT_TYPE_BOOL,
offsetof(StdRdOptions, user_catalog_table)},
{"parallel_workers", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, parallel_workers)}
+ offsetof(StdRdOptions, parallel_workers)},
+ {"max_pred_locks_per_relation", RELOPT_TYPE_INT,
+ offsetof(StdRdOptions, max_predicate_locks_per_relation)},
+ {"max_pred_locks_per_page", RELOPT_TYPE_INT,
+ offsetof(StdRdOptions, max_predicate_locks_per_page)}
};
options = parseRelOptions(reloptions, validate, kind, &numoptions);
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index ac927f2c3a..9b767d7e04 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -443,8 +443,9 @@ static void RestoreScratchTarget(bool lockheld);
static void RemoveTargetIfNoLongerUsed(PREDICATELOCKTARGET *target,
uint32 targettaghash);
static void DeleteChildTargetLocks(const PREDICATELOCKTARGETTAG *newtargettag);
-static int MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag);
-static bool CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag);
+static int MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag, Relation relation);
+static bool CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag,
+ Relation relation);
static void DecrementParentLocks(const PREDICATELOCKTARGETTAG *targettag);
static void CreatePredicateLock(const PREDICATELOCKTARGETTAG *targettag,
uint32 targettaghash,
@@ -453,7 +454,8 @@ static void DeleteLockTarget(PREDICATELOCKTARGET *target, uint32 targettaghash);
static bool TransferPredicateLocksToNewTarget(PREDICATELOCKTARGETTAG oldtargettag,
PREDICATELOCKTARGETTAG newtargettag,
bool removeOld);
-static void PredicateLockAcquire(const PREDICATELOCKTARGETTAG *targettag);
+static void PredicateLockAcquire(const PREDICATELOCKTARGETTAG *targettag,
+ Relation relation);
static void DropAllPredicateLocksFromTable(Relation relation,
bool transfer);
static void SetNewSxactGlobalXmin(void);
@@ -2138,17 +2140,27 @@ DeleteChildTargetLocks(const PREDICATELOCKTARGETTAG *newtargettag)
* entirely arbitrarily (and without benchmarking).
*/
static int
-MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag)
+MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag, Relation relation)
{
switch (GET_PREDICATELOCKTARGETTAG_TYPE(*tag))
{
case PREDLOCKTAG_RELATION:
+ {
+ int rel_max_pred_locks = RelationGetMaxPredicateLocksPerRelation(relation, -1);
+ if (rel_max_pred_locks != -1)
+ return rel_max_pred_locks;
return max_predicate_locks_per_relation < 0
? max_predicate_locks_per_xact / -max_predicate_locks_per_relation
: max_predicate_locks_per_relation;
+ }
case PREDLOCKTAG_PAGE:
+ {
+ int rel_max_pred_locks_per_page = RelationGetMaxPredicateLocksPerPage(relation, -1);
+ if (rel_max_pred_locks_per_page != -1)
+ return rel_max_pred_locks_per_page;
return max_predicate_locks_per_page;
+ }
case PREDLOCKTAG_TUPLE:
@@ -2174,7 +2186,8 @@ MaxPredicateChildLocks(const PREDICATELOCKTARGETTAG *tag)
* Returns true if a parent lock was acquired and false otherwise.
*/
static bool
-CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag)
+CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag,
+ Relation relation)
{
PREDICATELOCKTARGETTAG targettag,
nexttag,
@@ -2204,7 +2217,7 @@ CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag)
parentlock->childLocks++;
if (parentlock->childLocks >
- MaxPredicateChildLocks(&targettag))
+ MaxPredicateChildLocks(&targettag, relation))
{
/*
* We should promote to this parent lock. Continue to check its
@@ -2220,7 +2233,7 @@ CheckAndPromotePredicateLockRequest(const PREDICATELOCKTARGETTAG *reqtag)
if (promote)
{
/* acquire coarsest ancestor eligible for promotion */
- PredicateLockAcquire(&promotiontag);
+ PredicateLockAcquire(&promotiontag, relation);
return true;
}
else
@@ -2362,7 +2375,7 @@ CreatePredicateLock(const PREDICATELOCKTARGETTAG *targettag,
* any finer-grained locks covered by the new one.
*/
static void
-PredicateLockAcquire(const PREDICATELOCKTARGETTAG *targettag)
+PredicateLockAcquire(const PREDICATELOCKTARGETTAG *targettag, Relation relation)
{
uint32 targettaghash;
bool found;
@@ -2395,7 +2408,7 @@ PredicateLockAcquire(const PREDICATELOCKTARGETTAG *targettag)
* coarser granularity, or whether there are finer-granularity locks to
* clean up.
*/
- if (CheckAndPromotePredicateLockRequest(targettag))
+ if (CheckAndPromotePredicateLockRequest(targettag, relation))
{
/*
* Lock request was promoted to a coarser-granularity lock, and that
@@ -2431,7 +2444,7 @@ PredicateLockRelation(Relation relation, Snapshot snapshot)
SET_PREDICATELOCKTARGETTAG_RELATION(tag,
relation->rd_node.dbNode,
relation->rd_id);
- PredicateLockAcquire(&tag);
+ PredicateLockAcquire(&tag, relation);
}
/*
@@ -2455,7 +2468,7 @@ PredicateLockPage(Relation relation, BlockNumber blkno, Snapshot snapshot)
relation->rd_node.dbNode,
relation->rd_id,
blkno);
- PredicateLockAcquire(&tag);
+ PredicateLockAcquire(&tag, relation);
}
/*
@@ -2518,7 +2531,7 @@ PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snapshot)
relation->rd_id,
ItemPointerGetBlockNumber(tid),
ItemPointerGetOffsetNumber(tid));
- PredicateLockAcquire(&tag);
+ PredicateLockAcquire(&tag, relation);
}
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index ddad71a10f..ca75de7df6 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1919,6 +1919,8 @@ psql_completion(const char *text, int start, int end)
"autovacuum_vacuum_threshold",
"fillfactor",
"parallel_workers",
+ "max_pred_locks_per_relation",
+ "max_pred_locks_per_page",
"log_autovacuum_min_duration",
"toast.autovacuum_enabled",
"toast.autovacuum_freeze_max_age",
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index a617a7cf56..0e5722f6ed 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -277,6 +277,8 @@ typedef struct StdRdOptions
bool user_catalog_table; /* use as an additional catalog
* relation */
int parallel_workers; /* max number of parallel workers */
+ int max_predicate_locks_per_relation; /* max number of predicate locks */
+ int max_predicate_locks_per_page; /* max number of predicate locks per page */
} StdRdOptions;
#define HEAP_MIN_FILLFACTOR 10
@@ -325,6 +327,25 @@ typedef struct StdRdOptions
((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+/*
+ * RelationGetMaxPredicateLocksPerRelation
+ * Returns the relation's max_predicate_locks_per_relation reloption setting.
+ * Note multiple eval of argument!
+ */
+#define RelationGetMaxPredicateLocksPerRelation(relation, defaultmpl) \
+ ((relation)->rd_options ? \
+ ((StdRdOptions *) (relation)->rd_options)->max_predicate_locks_per_relation : (defaultmpl))
+
+/*
+ * RelationGetMaxPredicateLocksPerPage
+ * Returns the relation's max_predicate_locks_per_page reloption setting.
+ * Note multiple eval of argument!
+ */
+#define RelationGetMaxPredicateLocksPerPage(relation, defaultmplpp) \
+ ((relation)->rd_options ? \
+ ((StdRdOptions *) (relation)->rd_options)->max_predicate_locks_per_page : (defaultmplpp))
+
+
/*
* ViewOptions
* Contents of rd_options for views
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 27, 2017 at 7:35 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote: > Kevin Grittner <kgrittn@gmail.com> writes: > >> It occurred to me that it would make sense to allow these settings >> to be attached to a database or table (though *not* a role). I'll >> look at that when I get back if you don't get to it first. > > Attached is a draft patch (no docs or tests) on top of the previous one, > adding reloptions for the per-relation and per-page thresholds. That > made me realise that the corresponding GUCs don't need to be PGC_SIGHUP, > but could be PGC_SUSET or PGC_USERSET. I'll adjust the original patch > if you agree. I have not got around around to adding per-database > versions of the setting, but could have a stab at that. Unfortunately, I was unable to get the follow-on patch to allow setting by relation into a shape I liked. Let's see what we can do for the next release. The first patch was applied with only very minor tweaks. I realize that nothing would break if individual users could set their granularity thresholds on individual connections, but as someone who has filled the role of DBA, the thought kinda made my skin crawl. I left it as PGC_SIGHUP for now; we can talk about loosening that up later, but I think we should have one or more use-cases that outweigh the opportunities for confusion and bad choices by individual programmers to justify that. -- Kevin Grittner
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Kevin Grittner <kgrittn@gmail.com> writes: > Unfortunately, I was unable to get the follow-on patch to allow > setting by relation into a shape I liked. Let's see what we can do > for the next release. Okay, I'll try and crete a more comprehensive version of it for the next commitfest. > The first patch was applied with only very minor tweaks. Thanks! > I realize that nothing would break if individual users could set their > granularity thresholds on individual connections, but as someone who > has filled the role of DBA, the thought kinda made my skin crawl. I > left it as PGC_SIGHUP for now; we can talk about loosening that up > later, but I think we should have one or more use-cases that outweigh > the opportunities for confusion and bad choices by individual > programmers to justify that. I agree. The committed version is fine for my current use case. - ilmari -- "A disappointingly low fraction of the human race is,at any given time, on fire." - Stig Sandbeck Mathisen