diff --git a/src/Lock/BasicLockInformationProvider.php b/src/Lock/BasicLockInformationProvider.php index 15f4604..7d40d7b 100644 --- a/src/Lock/BasicLockInformationProvider.php +++ b/src/Lock/BasicLockInformationProvider.php @@ -22,8 +22,8 @@ public function getLockInformation() $hostname = gethostname(); $params = array(); - $params[] = $pid; - $params[] = $hostname; + $params['pid'] = $pid; + $params['hostname'] = $hostname; return $params; } diff --git a/src/Lock/PredisRedisLock.php b/src/Lock/PredisRedisLock.php index 4299a03..2632fe3 100644 --- a/src/Lock/PredisRedisLock.php +++ b/src/Lock/PredisRedisLock.php @@ -16,15 +16,20 @@ * * @author Kamil Dziedzic */ -class PredisRedisLock extends LockAbstract +class PredisRedisLock extends LockAbstract implements LockExpirationInterface { /** * Predis connection * - * @var + * @var Predis\Client */ protected $client; + /** + * @var int Expiration time of the lock in seconds + */ + protected $expiration = 0; + /** * @param $client Predis\Client */ @@ -35,6 +40,14 @@ public function __construct($client) $this->client = $client; } + /** + * @param int $expiration Expiration time of the lock in seconds + */ + public function setExpiration($expiration) + { + $this->expiration = $expiration; + } + /** * @param string $name * @param bool $blocking @@ -42,11 +55,56 @@ public function __construct($client) */ protected function getLock($name, $blocking) { - if (!$this->client->setnx($name, serialize($this->getLockInformation()))) { - return false; + /** + * Perform the process recommended by Redis for acquiring a lock, from here: https://redis.io/commands/setnx + * We are "C4" in this example... + * + * 1. C4 sends SETNX lock.foo in order to acquire the lock (sets the value if it does not already exist). + * 2. The crashed client C3 still holds it, so Redis will reply with 0 to C4. + * 3. C4 sends GET lock.foo to check if the lock expired. + * If it is not, it will sleep for some time and retry from the start. + * 4. Instead, if the lock is expired because the Unix time at lock.foo is older than the current Unix time, + * C4 tries to perform: + * GETSET lock.foo + * Because of the GETSET semantic, C4 can check if the old value stored at key is still an expired timestamp + * If it is, the lock was acquired. + * 5. If another client, for instance C5, was faster than C4 and acquired the lock with the GETSET operation, + * the C4 GETSET operation will return a non expired timestamp. + * C4 will simply restart from the first step. Note that even if C4 wrote they key and set the expiry time + * a few seconds in the future this is not a problem. C5's timeout will just be a few seconds later. + */ + + $lockValue = $this->getLockInformation(); + if ($this->expiration) { + // Add expiration timestamp to value stored in Redis. + $lockValue['expires'] = time() + $this->expiration; } + $lockValue = serialize($lockValue); - return true; + if ($this->client->setnx($name, $lockValue)) { + return true; + } + + // Check if the existing lock has an expiry time. If it does and it has expired, delete the lock. + if ($existingValue = $this->client->get($name)) { + $existingValue = unserialize($existingValue); + if (!empty($existingValue['expires']) && $existingValue['expires'] <= time()) { + // The existing lock has expired. We can delete it and take over. + $newExistingValue = unserialize($this->client->getset($name, $lockValue)); + + // GETSET atomically sets key to value and returns the old value that was stored at key. + // If the old value from getset does not still contain an expired timestamp + // another probably acquired the lock in the meantime. + if ($newExistingValue['expires'] > time()) { + return false; + } + + // Got it! + return true; + } + } + + return false; } /** @@ -57,7 +115,7 @@ protected function getLock($name, $blocking) */ public function releaseLock($name) { - if (isset($this->locks[$name]) && $this->client->del($name)) { + if (isset($this->locks[$name]) && $this->client->del(array($name))) { unset($this->locks[$name]); return true; @@ -76,4 +134,20 @@ public function isLocked($name) { return null !== $this->client->get($name); } + + /** + * Forget a lock without releasing it + * + * @param string $name name of lock + * @return bool + */ + public function clearLock($name) + { + if (!isset($this->locks[$name])) { + return false; + } + + unset($this->locks[$name]); + return true; + } } diff --git a/src/Lock/ResolvedHostnameLockInformationProvider.php b/src/Lock/ResolvedHostnameLockInformationProvider.php index 901b24c..c1d814e 100644 --- a/src/Lock/ResolvedHostnameLockInformationProvider.php +++ b/src/Lock/ResolvedHostnameLockInformationProvider.php @@ -21,7 +21,7 @@ class ResolvedHostnameLockInformationProvider extends BasicLockInformationProvid public function getLockInformation() { $params = parent::getLockInformation(); - $params[] = gethostbyname(gethostname()); + $params['hostIp'] = gethostbyname(gethostname()); return $params; } diff --git a/tests/Lock/PredisRedisLockTest.php b/tests/Lock/PredisRedisLockTest.php new file mode 100644 index 0000000..9232672 --- /dev/null +++ b/tests/Lock/PredisRedisLockTest.php @@ -0,0 +1,76 @@ +createPredisClient(); + $lock = $this->createLock($predis); + $mutex = new Mutex('very-critical-stuff', $lock); + $this->assertTrue($mutex->acquireLock()); + } + + public function testAcquireLockFails() + { + $predis = $this->createPredisClient(); + + // Acquire lock in 1st instance - should succeed + $lock = $this->createLock($predis); + $mutex = new Mutex('very-critical-stuff', $lock); + $this->assertTrue($mutex->acquireLock()); + + // Acquire lock in 2nd instance - should fail instantly because 0 timeout + $lock2 = $this->createLock($predis); + $mutex2 = new Mutex('very-critical-stuff', $lock2); + $this->assertFalse($mutex2->acquireLock(0)); + } + + public function testAcquireLockSucceedsAfterReleased() + { + $predis = $this->createPredisClient(); + + // Acquire lock in 1st instance - should succeed + $lock = $this->createLock($predis); + $mutex = new Mutex('very-critical-stuff', $lock); + $this->assertTrue($mutex->acquireLock()); + + $this->assertTrue($mutex->releaseLock()); + + // Acquire lock in 2nd instance - should succeed because 1st lock had been released + $lock2 = $this->createLock($predis); + $mutex2 = new Mutex('very-critical-stuff', $lock2); + $this->assertTrue($mutex2->acquireLock(0)); + } + + public function testAcquireLockSucceedsAfterTimeout() + { + $predis = $this->createPredisClient(); + + // Acquire lock in 1st instance - should succeed + $lock = $this->createLock($predis); + $lock->setExpiration(2); + $mutex = new Mutex('very-critical-stuff', $lock); + $this->assertTrue($mutex->acquireLock()); + + // Acquire lock in 2nd instance - should succeed after 2 seconds + $lock2 = $this->createLock($predis); + $mutex2 = new Mutex('very-critical-stuff', $lock2); + $this->assertTrue($mutex2->acquireLock()); + } +} diff --git a/tests/Mock/MockPredisClient.php b/tests/Mock/MockPredisClient.php index 766f462..e153522 100644 --- a/tests/Mock/MockPredisClient.php +++ b/tests/Mock/MockPredisClient.php @@ -70,16 +70,18 @@ public function get($key) } /** - * @param string $key + * @param string[] $keys * @return bool */ - public function del($key) + public function del(array $keys) { if (!$this->available) { return false; } - unset(self::$data[$key]); + foreach ($keys as $key) { + unset(self::$data[$key]); + } return true; } @@ -91,4 +93,43 @@ public function setAvailable($available) { $this->available = (bool)$available; } + + /** + * @param $key + * @param $value + * @param null $expireResolution + * @param null $expireTTL + * @param null $flag + * + * @return bool + */ + public function set($key, $value, $expireResolution = null, $expireTTL = null, $flag = null) + { + if (!$this->available) { + return false; + } + + self::$data[$key] = (string) $value; + + return true; + } + + /** + * @param $key + * @param $value + * + * @return string|null + */ + public function getset($key, $value) + { + if (!$this->available) { + return false; + } + + $oldValue = $this->get($key); + + $this->set($key, $value); + + return $oldValue; + } }