Skip to content
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

[8.x] phpredis lock serialization and compression support #36412

Merged
merged 1 commit into from
Mar 5, 2021
Merged
Show file tree
Hide file tree
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
89 changes: 89 additions & 0 deletions src/Illuminate/Cache/PhpRedisLock.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

namespace Illuminate\Cache;

use Illuminate\Redis\Connections\PhpRedisConnection;
use Redis;
use UnexpectedValueException;

class PhpRedisLock extends RedisLock
{
public function __construct(PhpRedisConnection $redis, string $name, int $seconds, ?string $owner = null)
{
parent::__construct($redis, $name, $seconds, $owner);
}

/**
* {@inheritDoc}
*/
public function release()
{
return (bool) $this->redis->eval(
LuaScripts::releaseLock(),
1,
$this->name,
$this->serializedAndCompressedOwner()
);
}

protected function serializedAndCompressedOwner(): string
{
$client = $this->redis->client();

/* If a serialization mode such as "php" or "igbinary" and/or a
* compression mode such as "lzf" or "zstd" is enabled, the owner
* must be serialized and/or compressed by us, because phpredis does
* not do this for the eval command.
*
* Name must not be modified!
*/
$owner = $client->_serialize($this->owner);
Copy link
Member

@taylorotwell taylorotwell Mar 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheLevti - So adding this call will not break any existing locks that were created before this change? I could imagine a scenario where locks exist - application is deployed with this changed - will those locks still be able to be regathered and released?

Copy link
Contributor Author

@TheLevti TheLevti Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, existing locks will work on deployment (if the lock owner is restored of course). If phpredis has serialization disabled, this will return the same value as passed down (See: https://github.com/phpredis/phpredis/blob/develop/library.c#L3029). I wrote a test for this case, which passes. (See: https://github.com/laravel/framework/pull/36412/files#diff-07d7fee8b1e505b0e2816e7b5dd26b90abc81614820689b5a070912f495c8279R31)

Copy link
Contributor Author

@TheLevti TheLevti Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing changes in regards of the redlock algorithm. The way the lock is created has not changed here. The same eval call is done with the same lua script executed on release as before. This change is all about modifying the passed arguments to the eval call when releasing the lock.


/* Once the phpredis extension exposes a compress function like the
* above `_serialize()` function, we should switch to it to guarantee
* consistency in the way the extension serializes and compresses to
* avoid the need to check each compression option ourselves.
*
* @see https://github.com/phpredis/phpredis/issues/1938
*/
if ($this->compressed()) {
if ($this->lzfCompressed()) {
$owner = \lzf_compress($owner);
} elseif ($this->zstdCompressed()) {
$owner = \zstd_compress($owner, $client->getOption(Redis::OPT_COMPRESSION_LEVEL));
} elseif ($this->lz4Compressed()) {
$owner = \lz4_compress($owner, $client->getOption(Redis::OPT_COMPRESSION_LEVEL));
} else {
throw new UnexpectedValueException(sprintf(
'Unknown phpredis compression in use (%d). Unable to release lock.',
$client->getOption(Redis::OPT_COMPRESSION)
));
}
}

return $owner;
}

protected function compressed(): bool
{
return $this->redis->client()->getOption(Redis::OPT_COMPRESSION) !== Redis::COMPRESSION_NONE;
}

protected function lzfCompressed(): bool
{
return defined('Redis::COMPRESSION_LZF') &&
$this->redis->client()->getOption(Redis::OPT_COMPRESSION) === Redis::COMPRESSION_LZF;
}

protected function zstdCompressed(): bool
{
return defined('Redis::COMPRESSION_ZSTD') &&
$this->redis->client()->getOption(Redis::OPT_COMPRESSION) === Redis::COMPRESSION_ZSTD;
}

protected function lz4Compressed(): bool
{
return defined('Redis::COMPRESSION_LZ4') &&
$this->redis->client()->getOption(Redis::OPT_COMPRESSION) === Redis::COMPRESSION_LZ4;
}
}
10 changes: 9 additions & 1 deletion src/Illuminate/Cache/RedisStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Contracts\Cache\LockProvider;
use Illuminate\Contracts\Redis\Factory as Redis;
use Illuminate\Redis\Connections\PhpRedisConnection;

class RedisStore extends TaggableStore implements LockProvider
{
Expand Down Expand Up @@ -188,7 +189,14 @@ public function forever($key, $value)
*/
public function lock($name, $seconds = 0, $owner = null)
{
return new RedisLock($this->lockConnection(), $this->prefix.$name, $seconds, $owner);
$lockName = $this->prefix.$name;
$lockConnection = $this->lockConnection();

if ($lockConnection instanceof PhpRedisConnection) {
return new PhpRedisLock($lockConnection, $lockName, $seconds, $owner);
}

return new RedisLock($lockConnection, $lockName, $seconds, $owner);
}

/**
Expand Down
Loading