Просмотр исходного кода

Client handles -NOSCRIPT falling back to EVAL for scripted commands.

Now Predis\Command\ScriptedCommand uses EVALSHA instead of EVAL internally
so that performances should be better since the client do not resend the
Lua script body on each call.

Plain EVALSHA commands are not affected and will return or throw the error.
Daniele Alessandri 13 лет назад
Родитель
Сommit
2d0b30dac5

+ 6 - 0
CHANGELOG.md

@@ -18,6 +18,12 @@ v0.8.0 (201x-xx-xx)
   Redis are not generated by connection classes anymore but are thrown by the
   Redis are not generated by connection classes anymore but are thrown by the
   client class and other abstractions such as pipeline contexts.
   client class and other abstractions such as pipeline contexts.
 
 
+- `Predis\Command\ScriptedCommand` internally relies on `EVALSHA` instead of
+  `EVAL` thus avoiding to send the Lua script body on each request. The client
+  automatically resends the command falling back to `EVAL` when Redis returns a
+  `-NOSCRIPT` error. Automatic fallback to `EVAL` does not work with pipelines
+  or when inside a MULTI / EXEC context or with plain `EVALSHA` commands.
+
 - Cluster and replication connections now extend a new common interface,
 - Cluster and replication connections now extend a new common interface,
   `Predis\Connection\AggregatedConnectionInterface`.
   `Predis\Connection\AggregatedConnectionInterface`.
 
 

+ 13 - 2
lib/Predis/Client.php

@@ -21,6 +21,7 @@ use Predis\PubSub\PubSubContext;
 use Predis\Monitor\MonitorContext;
 use Predis\Monitor\MonitorContext;
 use Predis\Pipeline\PipelineContext;
 use Predis\Pipeline\PipelineContext;
 use Predis\Transaction\MultiExecContext;
 use Predis\Transaction\MultiExecContext;
+use Predis\Command\ScriptedCommand;
 
 
 /**
 /**
  * Main class that exposes the most high-level interface to interact with Redis.
  * Main class that exposes the most high-level interface to interact with Redis.
@@ -209,7 +210,7 @@ class Client implements ClientInterface
         $response = $this->connection->executeCommand($command);
         $response = $this->connection->executeCommand($command);
 
 
         if ($response instanceof ResponseErrorInterface) {
         if ($response instanceof ResponseErrorInterface) {
-            $this->onResponseError($command, $response);
+            $response = $this->onResponseError($command, $response);
         }
         }
 
 
         return $response;
         return $response;
@@ -231,7 +232,7 @@ class Client implements ClientInterface
         $response = $this->connection->executeCommand($command);
         $response = $this->connection->executeCommand($command);
 
 
         if ($response instanceof ResponseErrorInterface) {
         if ($response instanceof ResponseErrorInterface) {
-            $this->onResponseError($command, $response);
+            return $this->onResponseError($command, $response);
         }
         }
 
 
         return $response;
         return $response;
@@ -242,13 +243,23 @@ class Client implements ClientInterface
      *
      *
      * @param CommandInterface $command The command that generated the error.
      * @param CommandInterface $command The command that generated the error.
      * @param ResponseErrorInterface $response The error response instance.
      * @param ResponseErrorInterface $response The error response instance.
+     * @return mixed
      */
      */
     protected function onResponseError(CommandInterface $command, ResponseErrorInterface $response)
     protected function onResponseError(CommandInterface $command, ResponseErrorInterface $response)
     {
     {
+        if ($command instanceof ScriptedCommand && $response->getErrorType() === 'NOSCRIPT') {
+            $eval = $this->createCommand('eval');
+            $eval->setArguments($command->getEvalArguments());
+
+            return $this->executeCommand($eval);
+        }
+
         if ($this->options->exceptions === true) {
         if ($this->options->exceptions === true) {
             $message = $response->getMessage();
             $message = $response->getMessage();
             throw new ServerException($message);
             throw new ServerException($message);
         }
         }
+
+        return $response;
     }
     }
 
 
     /**
     /**

+ 13 - 2
lib/Predis/Command/ScriptedCommand.php

@@ -18,7 +18,7 @@ namespace Predis\Command;
  * @link http://redis.io/commands/eval
  * @link http://redis.io/commands/eval
  * @author Daniele Alessandri <suppakilla@gmail.com>
  * @author Daniele Alessandri <suppakilla@gmail.com>
  */
  */
-abstract class ScriptedCommand extends ServerEval
+abstract class ScriptedCommand extends ServerEvalSHA
 {
 {
     /**
     /**
      * Gets the body of a Lua script.
      * Gets the body of a Lua script.
@@ -67,6 +67,17 @@ abstract class ScriptedCommand extends ServerEval
             $numkeys = count($arguments);
             $numkeys = count($arguments);
         }
         }
 
 
-        return array_merge(array($this->getScript(), $numkeys), $arguments);
+        return array_merge(array(sha1($this->getScript()), $numkeys), $arguments);
+    }
+
+    /**
+     * @return array
+     */
+    public function getEvalArguments()
+    {
+        $arguments = $this->getArguments();
+        $arguments[0] = $this->getScript();
+
+        return $arguments;
     }
     }
 }
 }

+ 25 - 0
tests/Predis/ClientTest.php

@@ -643,6 +643,31 @@ class ClientTest extends StandardTestCase
         $this->assertInstanceOf('Predis\Monitor\MonitorContext', $monitor = $client->monitor());
         $this->assertInstanceOf('Predis\Monitor\MonitorContext', $monitor = $client->monitor());
     }
     }
 
 
+    /**
+     * @group disconnected
+     */
+    public function testClientResendScriptedCommandUsingEvalOnNoScriptErrors()
+    {
+        $command = $this->getMockForAbstractClass('Predis\Command\ScriptedCommand');
+        $command->expects($this->once())
+                ->method('getScript')
+                ->will($this->returnValue('return redis.call(\'exists\', KEYS[1])'));
+
+        $connection = $this->getMock('Predis\Connection\SingleConnectionInterface');
+        $connection->expects($this->at(0))
+                   ->method('executeCommand')
+                   ->with($command)
+                   ->will($this->returnValue(new ResponseError('NOSCRIPT')));
+        $connection->expects($this->at(1))
+                   ->method('executeCommand')
+                   ->with($this->isInstanceOf('Predis\Command\ServerEval'))
+                   ->will($this->returnValue(true));
+
+        $client = new Client($connection);
+
+        $this->assertTrue($client->executeCommand($command));
+    }
+
     // ******************************************************************** //
     // ******************************************************************** //
     // ---- HELPER METHODS ------------------------------------------------ //
     // ---- HELPER METHODS ------------------------------------------------ //
     // ******************************************************************** //
     // ******************************************************************** //

+ 5 - 3
tests/Predis/Command/ScriptedCommandTest.php

@@ -19,6 +19,7 @@ use \PHPUnit_Framework_TestCase as StandardTestCase;
 class ScriptedCommandTest extends StandardTestCase
 class ScriptedCommandTest extends StandardTestCase
 {
 {
     const LUA_SCRIPT = 'return { KEYS[1], KEYS[2], ARGV[1], ARGV[2] }';
     const LUA_SCRIPT = 'return { KEYS[1], KEYS[2], ARGV[1], ARGV[2] }';
+    const LUA_SCRIPT_SHA1 = '6e07f61f502e36d123fe28523076af588f5c315e';
 
 
     /**
     /**
      * @group disconnected
      * @group disconnected
@@ -36,7 +37,8 @@ class ScriptedCommandTest extends StandardTestCase
                 ->will($this->returnValue(2));
                 ->will($this->returnValue(2));
         $command->setArguments($arguments);
         $command->setArguments($arguments);
 
 
-        $this->assertSame(array_merge(array(self::LUA_SCRIPT, 2), $arguments), $command->getArguments());
+
+        $this->assertSame(array_merge(array(self::LUA_SCRIPT_SHA1, 2), $arguments), $command->getArguments());
     }
     }
 
 
     /**
     /**
@@ -55,7 +57,7 @@ class ScriptedCommandTest extends StandardTestCase
                 ->will($this->returnValue(-2));
                 ->will($this->returnValue(-2));
         $command->setArguments($arguments);
         $command->setArguments($arguments);
 
 
-        $this->assertSame(array_merge(array(self::LUA_SCRIPT, 2), $arguments), $command->getArguments());
+        $this->assertSame(array_merge(array(self::LUA_SCRIPT_SHA1, 2), $arguments), $command->getArguments());
     }
     }
 
 
     /**
     /**
@@ -156,6 +158,6 @@ class ScriptedCommandTest extends StandardTestCase
                 ->will($this->returnValue(2));
                 ->will($this->returnValue(2));
         $command->setArguments($arguments);
         $command->setArguments($arguments);
 
 
-        $this->assertSame(sha1(self::LUA_SCRIPT), $command->getScriptHash());
+        $this->assertSame(self::LUA_SCRIPT_SHA1, $command->getScriptHash());
     }
     }
 }
 }