Merge pull request #32 from TheLevti/develop

Improve phpredis support

Author: Willem Stuursma-Ruwen

.travis.yml

@@ -9,6 +9,7 @@ cache:
 php:
   - 7.1
   - 7.2
+  - 7.3
   - nightly
 
 env: 
@@ -32,13 +33,15 @@ matrix:
     - php: nightly
 
 before_install:
+  - pecl update-channels
+  - yes | pecl install -f igbinary lzf redis
   - phpenv config-add tests/php-travis.ini
   - redis-server --port 63791 &
   - redis-server --port 63792 &
   - redis-server --port 63793 &
 
 install:
-  - composer install --no-scripts --no-suggest --no-interaction
+  - composer install --no-progress --no-scripts --no-suggest --no-interaction
 
 before_script:
   - mysql -e 'create database test;'

README.md

@@ -25,6 +25,8 @@ php-lock/lock follows semantic versioning. Read more on [semver.org][1].
  - Optionally the [php-pcntl][3] extension to enable locking with `flock()` without
    busy waiting in CLI scripts.
  - Optionally `flock()`, `ext-redis`, `ext-pdo_mysql`, `ext-pdo_sqlite`, `ext-pdo_pgsql` or `ext-memcached` can be used as a backend for locks. See examples below.
+ - If `ext-redis` is used for locking and is configured to use igbinary for serialization or
+   lzf for compression, additionally `ext-igbinary` and/or `ext-lzf` have to be installed.
 
 ----
 

classes/mutex/PHPRedisMutex.php

@@ -2,15 +2,19 @@
 
 namespace malkusch\lock\mutex;
 
-use Redis;
-use RedisException;
 use malkusch\lock\exception\LockAcquireException;
 use malkusch\lock\exception\LockReleaseException;
+use Redis;
+use RedisException;
 
 /**
  * Mutex based on the Redlock algorithm using the phpredis extension.
  *
- * This implementation requires at least phpredis-2.2.4.
+ * This implementation requires at least phpredis-4.0.0. If used together with
+ * the lzf extension, and phpredis is configured to use lzf compression, at
+ * least phpredis-4.3.0 is required! For reason, see github issue link.
+ *
+ * @see https://github.com/phpredis/phpredis/issues/1477
  *
  * @author Markus Malkusch <[email protected]>
  * @license WTFPL
@@ -23,12 +27,13 @@ class PHPRedisMutex extends RedisMutex
     /**
      * Sets the connected Redis APIs.
      *
-     * The Redis APIs needs to be connected yet. I.e. Redis::connect() was
+     * The Redis APIs needs to be connected. I.e. Redis::connect() was
      * called already.
      *
-     * @param Redis[] $redisAPIs The Redis connections.
-     * @param string  $name      The lock name.
-     * @param int     $timeout   The time in seconds a lock expires, default is 3.
+     * @param array<\Redis|\RedisCluster> $redisAPIs The Redis connections.
+     * @param string                      $name      The lock name.
+     * @param int                         $timeout   The time in seconds a lock expires after.
+     *                                               Default is 3.
      *
      * @throws \LengthException The timeout must be greater than 0.
      */
@@ -42,7 +47,7 @@ public function __construct(array $redisAPIs, string $name, int $timeout = 3)
      */
     protected function add($redisAPI, string $key, string $value, int $expire): bool
     {
-        /** @var Redis $redisAPI */
+        /** @var \Redis $redisAPI */
         try {
             //  Will set the key, if it doesn't exist, with a ttl of $expire seconds
             return $redisAPI->set($key, $value, ["nx", "ex" => $expire]);
@@ -55,18 +60,28 @@ protected function add($redisAPI, string $key, string $value, int $expire): bool
         }
     }
 
+    /**
+     * @param \Redis|\RedisCluster $redis The Redis or RedisCluster connection.
+     * @throws LockReleaseException
+     */
     protected function evalScript($redis, string $script, int $numkeys, array $arguments)
     {
-        /** @var Redis $redis */
-
-        /*
-         * If a serializion mode such as "php" or "igbinary" is enabled, the arguments must be serialized but the keys
-         * must not.
-         *
-         * @issue 14
-         */
-        for ($i = $numkeys, $iMax = \count($arguments); $i < $iMax; $i++) {
+        for ($i = $numkeys; $i < \count($arguments); $i++) {
+            /*
+             * If a serialization mode such as "php" or "igbinary" is enabled, the arguments must be
+             * serialized by us, because phpredis does not do this for the eval command.
+             *
+             * The keys must not be serialized.
+             */
             $arguments[$i] = $redis->_serialize($arguments[$i]);
+
+            /*
+             * If LZF compression is enabled for the redis connection and the runtime has the LZF
+             * extension installed, compress the arguments as the final step.
+             */
+            if ($this->hasLzfCompression($redis)) {
+                $arguments[$i] = lzf_compress($arguments[$i]);
+            }
         }
 
         try {
@@ -75,4 +90,19 @@ protected function evalScript($redis, string $script, int $numkeys, array $argum
             throw new LockReleaseException("Failed to release lock", 0, $e);
         }
     }
+
+    /**
+     * Determines if lzf compression is enabled for the given connection.
+     *
+     * @param  \Redis|\RedisCluster $redis Redis connection.
+     * @return bool TRUE if lzf compression is enabled, false otherwise.
+     */
+    private function hasLzfCompression($redis): bool
+    {
+        if (!\defined('Redis::COMPRESSION_LZF')) {
+            return false;
+        }
+
+        return Redis::COMPRESSION_LZF === $redis->getOption(Redis::OPT_COMPRESSION);
+    }
 }

composer.json

@@ -46,6 +46,8 @@
         "psr/log": "^1"
     },
     "require-dev": {
+        "ext-igbinary": "*",
+        "ext-lzf": "*",
         "ext-memcached": "*",
         "ext-pcntl": "*",
         "ext-pdo_mysql": "*",
@@ -62,9 +64,11 @@
         "squizlabs/php_codesniffer": "^3.3"
     },
     "suggest": {
+        "ext-igbinary": "To use this library with PHP Redis igbinary serializer enabled.",
+        "ext-lzf": "To use this library with PHP Redis lzf compression enabled.",
         "ext-pnctl": "Enables locking with flock without busy waiting in CLI scripts.",
-        "ext-sysvsem": "Enables locking using semaphores.",
         "ext-redis": "To use this library with the PHP Redis extension.",
+        "ext-sysvsem": "Enables locking using semaphores.",
         "predis/predis": "To use this library with predis."
     },
     "archive": {

tests/mutex/PHPRedisMutexTest.php

@@ -41,8 +41,7 @@ protected function setUp()
             $uri  = parse_url($redisUri);
 
             // original Redis::set and Redis::eval calls will reopen the connection
-            $connection = new class extends Redis
-            {
+            $connection = new class extends Redis {
                 private $is_closed = false;
 
                 public function close()
@@ -100,6 +99,9 @@ private function closeMinorityConnections()
         }
 
         $numberToClose = ceil(count($this->connections) / 2) - 1;
+        if (0 >= $numberToClose) {
+            return;
+        }
 
         foreach ((array) array_rand($this->connections, $numberToClose) as $keyToClose) {
             $this->connections[$keyToClose]->close();
@@ -132,27 +134,33 @@ public function testEvalScriptFails()
     }
 
     /**
-     * @param $serialization
-     * @dataProvider dpSerializationModes
+     * @dataProvider serializationAndCompressionModes
      */
-    public function testSynchronizedWorks($serialization)
+    public function testSerializersAndCompressors($serializer, $compressor)
     {
         foreach ($this->connections as $connection) {
-            $connection->setOption(Redis::OPT_SERIALIZER, $serialization);
+            $connection->setOption(Redis::OPT_SERIALIZER, $serializer);
+            $connection->setOption(Redis::OPT_COMPRESSION, $compressor);
         }
 
-        $this->assertSame("test", $this->mutex->synchronized(function (): string {
-            return "test";
-        }));
+        $this->assertSame(
+            "test",
+            $this->mutex->synchronized(function (): string {
+                return "test";
+            })
+        );
     }
 
     public function testResistantToPartialClusterFailuresForAcquiringLock()
     {
         $this->closeMinorityConnections();
 
-        $this->assertSame("test", $this->mutex->synchronized(function (): string {
-            return "test";
-        }));
+        $this->assertSame(
+            "test",
+            $this->mutex->synchronized(function (): string {
+                return "test";
+            })
+        );
     }
 
     public function testResistantToPartialClusterFailuresForReleasingLock()
@@ -163,21 +171,42 @@ public function testResistantToPartialClusterFailuresForReleasingLock()
         }));
     }
 
-    public function dpSerializationModes()
+    public function serializationAndCompressionModes()
     {
         if (!class_exists(Redis::class)) {
             return [];
         }
 
-        $serializers = [
-            [Redis::SERIALIZER_NONE],
-            [Redis::SERIALIZER_PHP],
+        $options = [
+            [Redis::SERIALIZER_NONE, Redis::COMPRESSION_NONE],
+            [Redis::SERIALIZER_PHP, Redis::COMPRESSION_NONE],
         ];
 
         if (defined("Redis::SERIALIZER_IGBINARY")) {
-            $serializers[] = [constant("Redis::SERIALIZER_IGBINARY")];
+            $options[] = [
+                constant("Redis::SERIALIZER_IGBINARY"),
+                Redis::COMPRESSION_NONE
+            ];
+        }
+
+        if (defined("Redis::COMPRESSION_LZF")) {
+            $options[] = [
+                Redis::SERIALIZER_NONE,
+                constant("Redis::COMPRESSION_LZF")
+            ];
+            $options[] = [
+                Redis::SERIALIZER_PHP,
+                constant("Redis::COMPRESSION_LZF")
+            ];
+
+            if (defined("Redis::SERIALIZER_IGBINARY")) {
+                $options[] = [
+                    constant("Redis::SERIALIZER_IGBINARY"),
+                    constant("Redis::COMPRESSION_LZF")
+                ];
+            }
         }
 
-        return $serializers;
+        return $options;
     }
 }

tests/php-travis.ini

@@ -1,2 +1 @@
 extension = "memcached.so"
-extension = "redis.so"