On Fri, Dec 25, 2015 at 1:45 AM, Jeff Janes <jeff.ja...@gmail.com> wrote:
> On Wed, Dec 23, 2015 at 9:40 PM, Jeff Janes <jeff.ja...@gmail.com> wrote:
> > On Wed, Sep 23, 2015 at 11:33 PM, Jeff Janes <jeff.ja...@gmail.com>
> >> On further thought, neither do I. The attached patch inverts
> >> ResolveRecoveryConflictWithLock to be called back from the lmgr code
> >> is it like ResolveRecoveryConflictWithBufferPin code. It does not try
> >> cancel the conflicting lock holders from the signal handler, rather it
> >> loops an extra time and cancels the transactions on the next call.
> >> It looks like the deadlock detection is adequately handled within
> >> lmgr code within the back-ends of the other parties to the deadlock,
> >> didn't do a timeout for deadlock detection purposes.
> > I was testing that this still applies to git HEAD. And it doesn't due
> > to the re-factoring of lock.h into lockdef.h. (And apparently it
> > never actually did, because that refactoring happened long before I
> > wrote this patch. I guess I must have done this work against
> > 9_5_STABLE.)
> > standby.h cannot include lock.h because standby.h is included
> > indirectly by pg_xlogdump. But it has to get LOCKTAG which is only in
> > lock.h.
> > Does this mean that standby.h also needs to get parts spun off into a
> > new standbydef.h that can be included from front-end code?
> That is how I've done it.
> The lock cancel patch applies over the header split patch.
Review Comments -
* If somebody wakes us between LWLockRelease and WaitLatch, the latch
--- 1081,1103 ----
LockTimeout is set, also enable the timeout for that. We can save a
* few cycles by enabling both
timeout sources in one call.
! if (!InHotStandby)
! if (LockTimeout > 0)
! EnableTimeoutParams timeouts;
! timeouts.id =
! timeouts.type = TMPARAM_AFTER;
.delay_ms = DeadlockTimeout;
! timeouts.id = LOCK_TIMEOUT;
timeouts.type = TMPARAM_AFTER;
! timeouts.delay_ms = LockTimeout;
If you are enabling the timeout only in non-hotstandby mode, then
later in code (in same function) it should be disabled under the same
condition, otherwise it will lead to assertion failure.
+ * Clear any timeout requests established above. We assume here that the
+ * Startup
process doesn't have any other outstanding timeouts than what
+ * this function
+ * uses. If
that stops being true, we could cancel the timeouts
+ * individually, but that'd be slower.
Comment seems to be misaligned.
+ /* forget any pending STANDBY_DEADLOCK_TIMEOUT request */
Is there any reason to disable deadlock timeout when it is not
enabled by ResolveRecoveryConflictWithLock()?