Thread: 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.camel@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. nm
On Tue, Mar 29, 2011 at 1:38 AM, Noah Misch <noah@leadboat.com> wrote: > A few years ago, this list had a brief conversation on $SUBJECT: > http://archives.postgresql.org/message-id/1215443493.4051.600.camel@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
On Tue, Mar 29, 2011 at 1:26 PM, Robert Haas <robertmhaas@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
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Mar 29, 2011 at 1:38 AM, Noah Misch <noah@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
On Tue, Mar 29, 2011 at 2:20 PM, Tom Lane <tgl@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
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 <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
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
On Tue, Mar 29, 2011 at 2:29 PM, Noah Misch <noah@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
On Wed, Mar 30, 2011 at 04:48:26PM -0400, Robert Haas wrote: > On Tue, Mar 29, 2011 at 2:29 PM, Noah Misch <noah@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.
Attachment
(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
Attachment
2011/6/17 Shigeru Hanada <shigeru.hanada@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