Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
Kevin Grittner 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
On Mon, Feb 27, 2017 at 7:35 AM, Dagfinn Ilmari Mannsåker wrote: > Kevin Grittner 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
Kevin Grittner 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?=
Date: Wed, 25 Jan 2017 00:13:53 +
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;
re
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
On Tue, Jan 24, 2017 at 3:32 AM, Kevin Grittner wrote: > I will need some time to consider that Moved to CF 2017-03 for now. The last patch sent still applies. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
On Mon, Jan 23, 2017 at 9:50 AM, Dagfinn Ilmari Mannsåker 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. >>> ilm...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
On Mon, Jan 23, 2017 at 9:50 AM, Dagfinn Ilmari Mannsåker wrote: > Kevin Grittner 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: > >>> ilm...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
Kevin Grittner 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: >> ilm...@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?= Date: Mon, 12 Dec 2016 17:57:33 + 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' + + max_pred_locks_per_relation (integer) + + max_pred_locks_per_relation configuration parameter + + + + +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 divided by +the absolute value of this setting. The default is -2, which keeps +the behaviour from previous versions of PostgreSQL. +This parameter can only be set in the postgresql.conf +file or on the server command li
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
On Thu, Jan 5, 2017 at 12:19 PM, Tom Lane wrote: > ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >> ilm...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > ilm...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
ilm...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
Kevin Grittner writes: > On Wed, Dec 14, 2016 at 8:16 AM, Dagfinn Ilmari Mannsåker > 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 with the consequences of." -- Skud's Meta-Law -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add GUCs for predicate lock promotion thresholds
On Wed, Dec 14, 2016 at 8:16 AM, Dagfinn Ilmari Mannsåker 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[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?= Date: Mon, 12 Dec 2016 17:57:33 + 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' + + max_pred_locks_per_relation (integer) + + max_pred_locks_per_relation configuration parameter + + + + +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 PostgreSQL it used to be hard-coded to half +of , and you might +want to raise this value if you raise that. This parameter can only +be set at server start. + + + + + + + max_pred_locks_per_page (integer) + + max_pred_locks_per_page configuration parameter + + + + +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. + + + + + 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 - . + , +and/or + . 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
