From e1505ff742140b04dc8ff8bbc1d9fb12ccdbc2a0 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 30 Sep 2016 06:36:35 -0700
Subject: [PATCH] Use MAX_LOCK_TTL in RedisLockManager to avoid premature lock
 expiration

App servers should not lower the redis structure TTL of locks set
from CLI scripts, which is what would typically happen before if
they touched the same keys. Instead, keep a high lock record TTL
and rely on the logical TTLs for expiry semantics.

Change-Id: I2948369845c47d2682a85dd0564673f3b813be83
---
 includes/libs/lockmanager/RedisLockManager.php | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/includes/libs/lockmanager/RedisLockManager.php b/includes/libs/lockmanager/RedisLockManager.php
index 6001705ff8..ea9dde7f2a 100644
--- a/includes/libs/lockmanager/RedisLockManager.php
+++ b/includes/libs/lockmanager/RedisLockManager.php
@@ -102,7 +102,7 @@ class RedisLockManager extends QuorumLockManager {
 <<<LUA
 			local failed = {}
 			-- Load input params (e.g. session, ttl, time of request)
-			local rSession, rTTL, rTime = unpack(ARGV)
+			local rSession, rTTL, rMaxTTL, rTime = unpack(ARGV)
 			-- Check that all the locks can be acquired
 			for i,requestKey in ipairs(KEYS) do
 				local _, _, rType, resourceKey = string.find(requestKey,"(%w+):(%w+)$")
@@ -133,7 +133,7 @@ class RedisLockManager extends QuorumLockManager {
 					local _, _, rType, resourceKey = string.find(requestKey,"(%w+):(%w+)$")
 					redis.call('hSet',resourceKey,rType .. ':' .. rSession,rTime + rTTL)
 					-- In addition to invalidation logic, be sure to garbage collect
-					redis.call('expire',resourceKey,rTTL)
+					redis.call('expire',resourceKey,rMaxTTL)
 				end
 			end
 			return failed
@@ -144,7 +144,8 @@ LUA;
 					[
 						$this->session, // ARGV[1]
 						$this->lockTTL, // ARGV[2]
-						time() // ARGV[3]
+						self::MAX_LOCK_TTL, // ARGV[3]
+						time() // ARGV[4]
 					]
 				),
 				count( $pathsByKey ) # number of first argument(s) that are keys
-- 
2.20.1