Re: [HACKERS] deadlock_timeout at PGC_SIGHUP?
2011/6/17 Shigeru Hanada shigeru.han...@gmail.com: (2011/06/12 6:43), Noah Misch wrote: On Wed, Mar 30, 2011 at 04:48:26PM -0400, Robert Haas wrote: Me neither. If making the deadlock timeout PGC_SUSET is independently useful, I don't object to doing that first, and then we can wait and see if anyone feels motivated to do more. Here's the patch for that. Not much to it. I've reviewed the patch following the article in the PostgreSQL wiki. It seems fine except that it needs to be rebased, so I'll mark this Ready for committers'. OK, committed. -- Robert Haas EnterpriseDB: 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] deadlock_timeout at PGC_SIGHUP?
(2011/06/12 6:43), Noah Misch wrote:
On Wed, Mar 30, 2011 at 04:48:26PM -0400, Robert Haas wrote:
Me neither. If making the deadlock timeout PGC_SUSET is independently
useful, I don't object to doing that first, and then we can wait and
see if anyone feels motivated to do more.
Here's the patch for that. Not much to it.
I've reviewed the patch following the article in the PostgreSQL wiki.
It seems fine except that it needs to be rebased, so I'll mark this
Ready for committers'. Please see below for details of my review.
Submission review
=
The patch is in context diff format, and can be applied with shifting
a hunk. I attached rebased patch.
The patch fixes the document about deadlock_timeout. Changing GUC
setting restriction would not need test.
Usability review
The purpose of the patch is to allow only superusers to change
deadlock_timeout GUC parameter. That seems to fit the conclusion of the
thread:
http://archives.postgresql.org/pgsql-hackers/2011-03/msg01727.php
Feature test
After applying the patch, non-superuser's attempt to change
deadlock_timeout is rejected with proper error:
ERROR: permission denied to set parameter deadlock_timeout
But superusers still can do that.
The fix for the document is fine, and it follows the wording used for
similar cases.
This patch doesn't need any support of external tools such as pg_dump
and psql.
Performance review
==
This patch would not cause any performance issue.
Coding review
=
The patch follows coding guidelines, and seems to have no portability
issue. It includes proper comment which describes why the parameter
should not be changed by non-superuser.
The patch produces no compiler warning for both binaries and documents.
Architecture review
===
AFAICS, this patch adopts the GUC parameter's standards.
Regards,
--
Shigeru Hanada
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e835e4b..7329281 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** dynamic_library_path = 'C:\tools\postgre
*** 5266,5272
practice. On a heavily loaded server you might want to raise it.
Ideally the setting should exceed your typical transaction time,
so as to improve the odds that a lock will be released before
! the waiter decides to check for deadlock.
/para
para
--- 5266,5273
practice. On a heavily loaded server you might want to raise it.
Ideally the setting should exceed your typical transaction time,
so as to improve the odds that a lock will be released before
! the waiter decides to check for deadlock. Only superusers can change
! this setting.
/para
para
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 92391ed..48ffe95 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*** static struct config_int ConfigureNamesI
*** 1532,1539
},
{
! /* This is PGC_SIGHUP so all backends have the same value. */
! {deadlock_timeout, PGC_SIGHUP, LOCK_MANAGEMENT,
gettext_noop(Sets the time to wait on a lock before
checking for deadlock.),
NULL,
GUC_UNIT_MS
--- 1532,1539
},
{
! /* This is PGC_SUSET to prevent hiding from log_lock_waits. */
! {deadlock_timeout, PGC_SUSET, LOCK_MANAGEMENT,
gettext_noop(Sets the time to wait on a lock before
checking for deadlock.),
NULL,
GUC_UNIT_MS
--
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] deadlock_timeout at PGC_SIGHUP?
On Wed, Mar 30, 2011 at 04:48:26PM -0400, Robert Haas wrote:
On Tue, Mar 29, 2011 at 2:29 PM, Noah Misch n...@leadboat.com wrote:
It's actually not
clear to me what the user could usefully do other than trying to
preserve his transaction by setting a high deadlock_timeout - what is
the use case, other than that?
The other major use case is reducing latency in deadlock-prone
transactions. ?By
reducing deadlock_timeout for some or all involved transactions, the error
will
arrive earlier.
Good point.
Is it worth thinking about having an explicit setting for deadlock
priority? ?That'd be more work, of course, and I'm not sure it it's
worth it, but it'd also provide stronger guarantees than you can get
with this proposal.
That is a better UI for the first use case. ?I have only twice wished to
tweak
deadlock_timeout: once for the use case you mention, another time for that
second use case. ?Given that, I wouldn't have minded a rough UI. ?If you'd
use
this often and assign more than two or three distinct priorities, you'd
probably
appreciate the richer UI. ?Not sure how many shops fall in that group.
Me neither. If making the deadlock timeout PGC_SUSET is independently
useful, I don't object to doing that first, and then we can wait and
see if anyone feels motivated to do more.
Here's the patch for that. Not much to it.
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3981969..f94782f 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 5258,5264 dynamic_library_path =
'C:\tools\postgresql;H:\my_project\lib;$libdir'
practice. On a heavily loaded server you might want to raise it.
Ideally the setting should exceed your typical transaction time,
so as to improve the odds that a lock will be released before
! the waiter decides to check for deadlock.
/para
para
--- 5258,5265
practice. On a heavily loaded server you might want to raise it.
Ideally the setting should exceed your typical transaction time,
so as to improve the odds that a lock will be released before
! the waiter decides to check for deadlock. Only superusers can change
! this setting.
/para
para
diff --git a/src/backend/utils/index 92391ed..48ffe95 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***
*** 1532,1539 static struct config_int ConfigureNamesInt[] =
},
{
! /* This is PGC_SIGHUP so all backends have the same value. */
! {deadlock_timeout, PGC_SIGHUP, LOCK_MANAGEMENT,
gettext_noop(Sets the time to wait on a lock before
checking for deadlock.),
NULL,
GUC_UNIT_MS
--- 1532,1539
},
{
! /* This is PGC_SUSET to prevent hiding from log_lock_waits. */
! {deadlock_timeout, PGC_SUSET, LOCK_MANAGEMENT,
gettext_noop(Sets the time to wait on a lock before
checking for deadlock.),
NULL,
GUC_UNIT_MS
--
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] deadlock_timeout at PGC_SIGHUP?
On Tue, Mar 29, 2011 at 2:29 PM, Noah Misch n...@leadboat.com wrote: It's actually not clear to me what the user could usefully do other than trying to preserve his transaction by setting a high deadlock_timeout - what is the use case, other than that? The other major use case is reducing latency in deadlock-prone transactions. By reducing deadlock_timeout for some or all involved transactions, the error will arrive earlier. Good point. Is it worth thinking about having an explicit setting for deadlock priority? That'd be more work, of course, and I'm not sure it it's worth it, but it'd also provide stronger guarantees than you can get with this proposal. That is a better UI for the first use case. I have only twice wished to tweak deadlock_timeout: once for the use case you mention, another time for that second use case. Given that, I wouldn't have minded a rough UI. If you'd use this often and assign more than two or three distinct priorities, you'd probably appreciate the richer UI. Not sure how many shops fall in that group. Me neither. If making the deadlock timeout PGC_SUSET is independently useful, I don't object to doing that first, and then we can wait and see if anyone feels motivated to do more. (Of course, we're speaking of 9.2.) -- Robert Haas EnterpriseDB: 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] deadlock_timeout at PGC_SIGHUP?
On Tue, Mar 29, 2011 at 1:38 AM, Noah Misch n...@leadboat.com wrote: A few years ago, this list had a brief conversation on $SUBJECT: http://archives.postgresql.org/message-id/1215443493.4051.600.ca...@ebony.site What is notable/surprising about the behavior when two backends have different values for deadlock_timeout? After sleeping to acquire a lock, each backend will scan for deadlocks every time its own deadlock_timeout elapses. Some might be surprised that a larger-deadlock_timeout backend can still be the one to give up; consider this timeline: Backend Time Command A N/A SET deadlock_timeout = 1000 B N/A SET deadlock_timeout = 100 A 0 LOCK t B 50 LOCK u A 100 LOCK u B 1050 LOCK t (Backend A gets an ERROR at time 1100) More generally, one cannot choose deadlock_timeout values for two sessions such that a specific session will _always_ get the ERROR. However, one can drive the probability rather high. Compare to our current lack of control. Is some other behavior that only arises when backends have different deadlock_timeout settings more surprising than that one? If we could relax deadlock_timeout to a GucContext below PGC_SIGHUP, it would probably need to stop at PGC_SUSET for now. Otherwise, an unprivileged user could increase deadlock_timeout to hide his lock waits from log_lock_waits. One could remove that limitation by introducing a separate log_lock_waits timeout, but that patch would be significantly meatier. Some might also object to PGC_USERSET on the basis that a user could unfairly preserve his transaction by setting a high deadlock_timeout. However, that user could accomplish a similar denial of service by idly holding locks or trying deadlock-prone lock acquisitions in subtransactions. I'd be inclined to think that PGC_SUSET is plenty. It's actually not clear to me what the user could usefully do other than trying to preserve his transaction by setting a high deadlock_timeout - what is the use case, other than that? Is it worth thinking about having an explicit setting for deadlock priority? That'd be more work, of course, and I'm not sure it it's worth it, but it'd also provide stronger guarantees than you can get with this proposal. -- Robert Haas EnterpriseDB: 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] deadlock_timeout at PGC_SIGHUP?
On Tue, Mar 29, 2011 at 1:26 PM, Robert Haas robertmh...@gmail.com wrote: Is it worth thinking about having an explicit setting for deadlock priority? That'd be more work, of course, and I'm not sure it it's worth it, but it'd also provide stronger guarantees than you can get with this proposal. Priority makes better sense, I think. That's what we're trying to control after all. But you would need to change the way the deadlock detector works... -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] deadlock_timeout at PGC_SIGHUP?
Robert Haas robertmh...@gmail.com writes: On Tue, Mar 29, 2011 at 1:38 AM, Noah Misch n...@leadboat.com wrote: What is notable/surprising about the behavior when two backends have different values for deadlock_timeout? I'd be inclined to think that PGC_SUSET is plenty. It's actually not clear to me what the user could usefully do other than trying to preserve his transaction by setting a high deadlock_timeout - what is the use case, other than that? Yeah, that was my reaction too: what is the use case for letting different backends have different settings? It fails to give any real guarantees about who wins a deadlock, and I can't see any other reason for wanting session-specific settings. I don't know how difficult a priority setting would be. IIRC, the current deadlock detector always kills the process that detected the deadlock, but I *think* that's just a random choice and not an essential feature. If so, it'd be pretty easy to instead kill the lowest-priority xact among those involved in the deadlock. 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] deadlock_timeout at PGC_SIGHUP?
On Tue, Mar 29, 2011 at 2:20 PM, Tom Lane t...@sss.pgh.pa.us wrote: IIRC, the current deadlock detector always kills the process that detected the deadlock, but I *think* that's just a random choice and not an essential feature. If so, it'd be pretty easy to instead kill the lowest-priority xact among those involved in the deadlock. I think it was just easier. To kill yourself you just exit with an error. To kill someone else you have to deliver a signal and hope the other process exits cleanly. There are a bunch of things to wonder about too. If you don't kill yourself then you might still be in a deadlock cycle so presumably you have to reset the deadlock timer? What if two backends both decide to kill the same other backend. Does it handle getting a spurious signal late cleanly? How does it know which transaction the signal was for? Alternatively we could have the deadlock timer reset all the time and fire repeatedly. Then we could just have all backends ignore the deadlock if they're not the lowest priority session in the cycle. But this depends on everyone knowing everyone else's priority (and having a consistent view of it). -- greg -- 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] deadlock_timeout at PGC_SIGHUP?
Excerpts from Greg Stark's message of mar mar 29 11:15:50 -0300 2011: Alternatively we could have the deadlock timer reset all the time and fire repeatedly. Then we could just have all backends ignore the deadlock if they're not the lowest priority session in the cycle. But this depends on everyone knowing everyone else's priority (and having a consistent view of it). Presumably it'd be published in MyProc before going to sleep, so it'd be available for everyone and always consistent. Not sure if this requires extra locking, though. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] deadlock_timeout at PGC_SIGHUP?
On Tue, Mar 29, 2011 at 08:26:44AM -0400, Robert Haas wrote: I'd be inclined to think that PGC_SUSET is plenty. Agreed. A superuser who would have liked PGC_USERSET can always provide a SECURITY DEFINER function. It's actually not clear to me what the user could usefully do other than trying to preserve his transaction by setting a high deadlock_timeout - what is the use case, other than that? The other major use case is reducing latency in deadlock-prone transactions. By reducing deadlock_timeout for some or all involved transactions, the error will arrive earlier. Is it worth thinking about having an explicit setting for deadlock priority? That'd be more work, of course, and I'm not sure it it's worth it, but it'd also provide stronger guarantees than you can get with this proposal. That is a better UI for the first use case. I have only twice wished to tweak deadlock_timeout: once for the use case you mention, another time for that second use case. Given that, I wouldn't have minded a rough UI. If you'd use this often and assign more than two or three distinct priorities, you'd probably appreciate the richer UI. Not sure how many shops fall in that group. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] deadlock_timeout at PGC_SIGHUP?
A few years ago, this list had a brief conversation on $SUBJECT: http://archives.postgresql.org/message-id/1215443493.4051.600.ca...@ebony.site What is notable/surprising about the behavior when two backends have different values for deadlock_timeout? After sleeping to acquire a lock, each backend will scan for deadlocks every time its own deadlock_timeout elapses. Some might be surprised that a larger-deadlock_timeout backend can still be the one to give up; consider this timeline: Backend TimeCommand A N/A SET deadlock_timeout = 1000 B N/A SET deadlock_timeout = 100 A 0 LOCK t B 50 LOCK u A 100 LOCK u B 1050LOCK t (Backend A gets an ERROR at time 1100) More generally, one cannot choose deadlock_timeout values for two sessions such that a specific session will _always_ get the ERROR. However, one can drive the probability rather high. Compare to our current lack of control. Is some other behavior that only arises when backends have different deadlock_timeout settings more surprising than that one? If we could relax deadlock_timeout to a GucContext below PGC_SIGHUP, it would probably need to stop at PGC_SUSET for now. Otherwise, an unprivileged user could increase deadlock_timeout to hide his lock waits from log_lock_waits. One could remove that limitation by introducing a separate log_lock_waits timeout, but that patch would be significantly meatier. Some might also object to PGC_USERSET on the basis that a user could unfairly preserve his transaction by setting a high deadlock_timeout. However, that user could accomplish a similar denial of service by idly holding locks or trying deadlock-prone lock acquisitions in subtransactions. nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] deadlock_timeout
Why is deadlock_timeout set at SIGHUP? If it effects statement behaviour it ought to be a USERSET. Using CPU time isn't a problem we protect against anywhere else. I'd like to be able to set deadlock-prone transactions lower, yet keep a fairly high setting for others. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] deadlock_timeout
Simon Riggs [EMAIL PROTECTED] writes: Why is deadlock_timeout set at SIGHUP? Because it's not clear what the behavior would be like if different backends had different settings ... except that it'd probably be surprising. 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] deadlock_timeout
On Mon, 2008-07-07 at 11:16 -0400, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Why is deadlock_timeout set at SIGHUP? Because it's not clear what the behavior would be like if different backends had different settings ... except that it'd probably be surprising. Yeh, agreed. I was thinking to set the deadlock_timeout the same for all people running the deadlock-prone transactions and set it higher for others. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] deadlock_timeout
Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Why is deadlock_timeout set at SIGHUP? Because it's not clear what the behavior would be like if different backends had different settings ... except that it'd probably be surprising. I have added a code comment explaining this. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
