Re: [PATCH] Add GUCs for predicate lock promotion thresholds

From: ilmari(at)ilmari(dot)org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=)
To: Kevin Grittner <kgrittn(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers\(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add GUCs for predicate lock promotion thresholds
Date: 2017-01-23 15:50:05
Message-ID: d8jinp5bwsy.fsf@dalvik.ping.uio.no
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Kevin Grittner <kgrittn(at)gmail(dot)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(at)ilmari(dot)org (Dagfinn Ilmari Mannsker) 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

Attachment Content-Type Size
0001-Add-GUCs-for-predicate-lock-promotion-thresholds-v2.patch text/x-diff 9.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kuntal Ghosh 2017-01-23 15:58:32 Re: Add pgstathashindex() to get hash index table statistics.
Previous Message valeriof 2017-01-23 15:49:09 Deployment of an output plugin in Unix-like environment