diff --git a/src/Cm/RedisSession/Handler.php b/src/Cm/RedisSession/Handler.php index 5ce45e1..a60cb79 100644 --- a/src/Cm/RedisSession/Handler.php +++ b/src/Cm/RedisSession/Handler.php @@ -114,7 +114,7 @@ class Handler implements \SessionHandlerInterface /** * Try to break lock for at most this many seconds */ - const DEFAULT_FAIL_AFTER = 15; + const DEFAULT_FAIL_AFTER_SECONDS = 15; /** * The session lifetime for non-bots on the first write @@ -189,12 +189,17 @@ class Handler implements \SessionHandlerInterface /** * @var int */ - protected $_breakAfter; + protected $_breakAfterLockTries; /** * @var int */ - protected $_failAfter; + protected $_failAfterSeconds; + + /** + * @var int + */ + protected $_failAfterLockBreakTries; /** * @var boolean @@ -288,13 +293,13 @@ public function __construct(ConfigInterface $config, LoggerInterface $logger, $r $this->_compressionThreshold = $this->config->getCompressionThreshold() ?: self::DEFAULT_COMPRESSION_THRESHOLD; $this->_compressionLibrary = $this->config->getCompressionLibrary() ?: self::DEFAULT_COMPRESSION_LIBRARY; $this->_maxConcurrency = $this->config->getMaxConcurrency() ?: self::DEFAULT_MAX_CONCURRENCY; - $this->_failAfter = $this->config->getFailAfter() ?: self::DEFAULT_FAIL_AFTER; + $this->_failAfterSeconds = $this->config->getFailAfter() ?: self::DEFAULT_FAIL_AFTER_SECONDS; $this->_maxLifetime = $this->config->getMaxLifetime() ?: self::DEFAULT_MAX_LIFETIME; $this->_minLifetime = $this->config->getMinLifetime() ?: self::DEFAULT_MIN_LIFETIME; $this->_useLocking = ! $this->config->getDisableLocking(); - // Use sleep time multiplier so fail after time is in seconds - $this->_failAfter = (int) round((1000000 / self::SLEEP_TIME) * $this->_failAfter); + // Multiply fail after seconds by number of tries per second to get number of write lock breaking tries + $this->_failAfterLockBreakTries = (int) round($this->_getLockTriesPerSecond() * $this->_failAfterSeconds); // Sentinel config $sentinelServers = $this->config->getSentinelServers(); @@ -311,54 +316,54 @@ public function __construct(ConfigInterface $config, LoggerInterface $logger, $r $sentinel = NULL; $exception = NULL; for ($i = 0; $i <= $sentinelConnectRetries; $i++) // Try to connect to sentinels in round-robin fashion - foreach ($servers as $server) { - try { - $sentinelClient = new \Credis_Client($server, NULL, $timeout, $persistent); - $sentinelClient->forceStandalone(); - $sentinelClient->setMaxConnectRetries(0); - if ($sentinelPassword) { - try { - $sentinelClient->auth($sentinelPassword); - } catch (\CredisException $e) { - // Prevent throwing exception if Sentinel has no password set (error messages are different between redis 5 and redis 6) - if ($e->getCode() !== 0 || ( - strpos($e->getMessage(), 'ERR Client sent AUTH, but no password is set') === false && - strpos($e->getMessage(), 'ERR AUTH called without any password configured for the default user. Are you sure your configuration is correct?') === false) - ) { - throw $e; + foreach ($servers as $server) { + try { + $sentinelClient = new \Credis_Client($server, NULL, $timeout, $persistent); + $sentinelClient->forceStandalone(); + $sentinelClient->setMaxConnectRetries(0); + if ($sentinelPassword) { + try { + $sentinelClient->auth($sentinelPassword); + } catch (\CredisException $e) { + // Prevent throwing exception if Sentinel has no password set (error messages are different between redis 5 and redis 6) + if ($e->getCode() !== 0 || ( + strpos($e->getMessage(), 'ERR Client sent AUTH, but no password is set') === false && + strpos($e->getMessage(), 'ERR AUTH called without any password configured for the default user. Are you sure your configuration is correct?') === false) + ) { + throw $e; + } } } - } - - $sentinel = new \Credis_Sentinel($sentinelClient); - $sentinel - ->setClientTimeout($timeout) - ->setClientPersistent($persistent); - $redisMaster = $sentinel->getMasterClient($sentinelMaster); - if ($pass) $redisMaster->auth($pass); - - // Verify connected server is actually master as per Sentinel client spec - if ($sentinelVerifyMaster) { - $roleData = $redisMaster->role(); - if ( ! $roleData || $roleData[0] != 'master') { - usleep(100000); // Sleep 100ms and try again - $redisMaster = $sentinel->getMasterClient($sentinelMaster); - if ($pass) $redisMaster->auth($pass); + + $sentinel = new \Credis_Sentinel($sentinelClient); + $sentinel + ->setClientTimeout($timeout) + ->setClientPersistent($persistent); + $redisMaster = $sentinel->getMasterClient($sentinelMaster); + if ($pass) $redisMaster->auth($pass); + + // Verify connected server is actually master as per Sentinel client spec + if ($sentinelVerifyMaster) { $roleData = $redisMaster->role(); if ( ! $roleData || $roleData[0] != 'master') { - throw new \Exception('Unable to determine master redis server.'); + usleep(100000); // Sleep 100ms and try again + $redisMaster = $sentinel->getMasterClient($sentinelMaster); + if ($pass) $redisMaster->auth($pass); + $roleData = $redisMaster->role(); + if ( ! $roleData || $roleData[0] != 'master') { + throw new \Exception('Unable to determine master redis server.'); + } } } - } - if ($this->_dbNum || $persistent) $redisMaster->select(0); + if ($this->_dbNum || $persistent) $redisMaster->select(0); - $this->_redis = $redisMaster; - break 2; - } catch (\Exception $e) { - unset($sentinelClient); - $exception = $e; + $this->_redis = $redisMaster; + break 2; + } catch (\Exception $e) { + unset($sentinelClient); + $exception = $e; + } } - } unset($sentinel); if ( ! $this->_redis) { @@ -386,6 +391,16 @@ public function __construct(ConfigInterface $config, LoggerInterface $logger, $r ); } + /** + * Get number of write lock tries ran per second + * + * @return int + */ + protected function _getLockTriesPerSecond(): int + { + return 1000000 / self::SLEEP_TIME; + } + /** * Open session * @@ -455,7 +470,7 @@ public function read($sessionId) $tries = $waiting = $lock = 0; $lockPid = $oldLockPid = null; // Restart waiting for lock when current lock holder changes $detectZombies = false; - $breakAfter = $this->_getBreakAfter(); + $breakAfterTries = $this->_getLockTriesBeforeBreak(); $timeStart = microtime(true); $this->_log(sprintf("Attempting to take lock on ID %s", $sessionId)); @@ -467,14 +482,14 @@ public function read($sessionId) $lock = $this->_redis->hIncrBy($sessionId, 'lock', 1); // Get the pid of the process that has the lock - if ($lock != 1 && $tries + 1 >= $breakAfter) { + if ($lock != 1 && $tries + 1 >= $breakAfterTries) { $lockPid = $this->_redis->hGet($sessionId, 'pid'); } // If we got the lock, update with our pid and reset lock and expiration if ( $lock == 1 // We actually do have the lock || ( - $tries >= $breakAfter // We are done waiting and want to start trying to break it + $tries >= $breakAfterTries // We are done waiting and want to start trying to break it && $oldLockPid == $lockPid // Nobody else got the lock while we were waiting ) ) { @@ -571,7 +586,7 @@ public function read($sessionId) } } // Timeout - if ($tries >= $breakAfter + $this->_failAfter) { + if ($tries >= $breakAfterTries + $this->_failAfterSeconds) { $this->_hasLock = false; $this->_log( sprintf( @@ -628,7 +643,7 @@ public function read($sessionId) $this->_log( sprintf( "Successfully broke lock for ID %s after %.5f seconds (%d attempts). Lock: %d\nLast request of " - . "broken lock: %s", + . "broken lock: %s", $sessionId, (microtime(true) - $timeStart), $tries, @@ -881,14 +896,14 @@ protected function _writeRawSession($id, $data, $lifetime) { $sessionId = 'sess_' . $id; $this->_redis->pipeline() - ->select($this->_dbNum) - ->hMSet($sessionId, array( - 'data' => $this->_encodeData($data), - 'lock' => 0, // 0 so that next lock attempt will get 1 - )) - ->hIncrBy($sessionId, 'writes', 1) - ->expire($sessionId, min((int)$lifetime, (int)$this->_maxLifetime)) - ->exec(); + ->select($this->_dbNum) + ->hMSet($sessionId, array( + 'data' => $this->_encodeData($data), + 'lock' => 0, // 0 so that next lock attempt will get 1 + )) + ->hIncrBy($sessionId, 'writes', 1) + ->expire($sessionId, min((int)$lifetime, (int)$this->_maxLifetime)) + ->exec(); } /** @@ -917,20 +932,21 @@ protected function _pidExists($pid) } /** - * Get break time, calculated later than other config settings due to requiring session name to be set + * Get number of lock attempts to try before breaking lock. + * This is calculated later than other config settings due to requiring session name to be set * * @return int */ - protected function _getBreakAfter() + protected function _getLockTriesBeforeBreak() { // Has break after already been calculated? Only fetch from config once, then reuse variable. - if (!$this->_breakAfter) { + if (!$this->_breakAfterLockTries) { // Fetch relevant setting from config using session name - $this->_breakAfter = (float)($this->config->getBreakAfter() ?: self::DEFAULT_BREAK_AFTER); - // Use sleep time multiplier so break time is in seconds - $this->_breakAfter = (int)round((1000000 / self::SLEEP_TIME) * $this->_breakAfter); + $breakAfterSeconds = (float)($this->config->getBreakAfter() ?: self::DEFAULT_BREAK_AFTER); + // Multiply break after seconds by lock tries per second to get lock tries before break + $this->_breakAfterLockTries = (int)round($this->_getLockTriesPerSecond() * $breakAfterSeconds); } - return $this->_breakAfter; + return $this->_breakAfterLockTries; } }