Skip to content

Fixes bad variable names & property docs; extract code to a function #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 82 additions & 66 deletions src/Cm/RedisSession/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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 <password> 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 <password> 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) {
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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));

Expand All @@ -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
)
) {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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;
}
}