Jan Wieck wrote:
> Attached is a new patch that addresses most of the points raised
> in discussion before.
>
> 1) Most of the configuration variables are derived from
> deadlock_timeout now. The "check for conflicting lock request"
> interval is deadlock_timeout/10, clamped to 10ms. The "try to
> acquire exclusive lock" interval is deadlock_timeout/20, also
> clamped to 10ms. The only GUC variable remaining is
> autovacuum_truncate_lock_try=2000ms with a range from 0 (just try
> once) to 20000ms.
If we're going to keep this GUC, we need docs for it.
> I'd like to point out that this is a significant change in
> functionality as without the config option for the check
> interval, there is no longer any possibility to disable the call
> to LockHasWaiters() and return to the original (deadlock code
> kills autovacuum) behavior.
Arguably we could simplify the deadlock resolution code a little,
but it seems like it is probably safer to leave it as a failsafe,
at least for now.
> I did not touch the part about suppressing the stats and the
> ANALYZE step of "auto vacuum+analyze". The situation is no
> different today. When the deadlock code kills autovacuum, stats
> aren't updated either. And this patch is meant to cause
> autovacuum to finish the truncate in a few minutes or hours, so
> that the situation fixes itself.
Unless someone want to argue for more aggressive updating of the
stats, I guess I'll yield to that argument.
Besides the documentation, the only thing I could spot on this
go-around was that this comment probably no longer has a reason for
being since this is no longer conditional and what it's doing is
obvious from the code:
/* Initialize the starttime if we check for conflicting lock requests */
With docs and dropping the obsolete comment, I think this will be
ready.
-Kevin
