More PHP woes

[Pete Warden]

I've been running some more tests using the new PHPRedis build. It's great work, exactly what I need, but I'm now hitting some mysterious and intermittent problems.

I was seeing empty strings being occasionally returned for keys where I knew the value had been stored, once in every few thousand gets, but on a different key every run. I set up a simple sanity test:
        $relationshipsstring = $redis->get($mydbkey);
        $secondget = $redis->get($mydbkey);
        if ($relationshipsstring!=$secondget)
            die("Urk, random data! '$relationshipsstring' vs '$secondget'\n");
and confirmed this would fail randomly.

To debug this, I instrumented PHPRedis/redis.c so that all the socket reads and writes were logged. This is what I'm seeing just before a failure:

redis_sock_read:+OK
redis_sock_write:GET prd_8b96bd460b53aed22f1040c70982b9dd
redis_sock_read:-ERR unknown command
redis_sock_write:GET prd_8b96bd460b53aed22f1040c70982b9dd
redis_sock_read:$1222

I'm running 1.02 on OS X 10.6 Intel. Does this ring any bells with anyone? Do you have any suggestions for debugging this myself, or getting info that could help someone else understand what's happening?

thanks,
            Pete

[Pete Warden]

The new PHPRedis code fails when a value 1,000 or 1,000,000 characters
in length is set. This is due to an incorrect calculation of the
number of digits for the SET command, so that what should be

SET key 1000
<1000 character-long value>

is actually sent to Redis as

SET key 100
<1000 character-long value>

In the second case, the remaining 900 characters remain in the buffer
and are misinterpreted as a command, meaning the next command gets a -
ERR result, no matter what it was.

Here's a patch to the PHPRedis code that fixes this:

----------------------------------------
diff --git a/redis.c b/redis.c
index efdd2fd..ac3f21f 100755
--- a/redis.c
+++ b/redis.c
@@ -127,7 +127,7 @@ redis_cmd_format(char **ret, char *format, ...) {
va_list ap;

int total = 0, sz, ret_sz;
- int i;
+ int i, ci;
unsigned int u;


@@ -153,7 +153,12 @@ redis_cmd_format(char **ret, char *format, ...) {
case 'd':
i = va_arg(ap, int);
/* compute display size of integer value */
- sz = 1+log(abs(i))/log(10);
+ sz = 0;
+ ci = abs(i);
+ while (ci>0) {
+ ci = (ci/10);
+ sz += 1;
+ }
if(i == 0) { /* log 0 doesn't make sense. */
sz = 1;
} else if(i < 0) { /* allow for neg sign as
well. */
-------------------------------

and here's a test case that fails with the old code, and works with
the new:

-------------------------------
#!/usr/bin/php
<?php

$valuelength = 1000;
$testvalue = str_pad('', $valuelength, '0');

$redis = new Redis();

$redis->connect("127.0.0.1", 6379);

$redis->set('key', $testvalue);
$currentvalue = $redis->get('key');
if ($currentvalue!==$testvalue)
die("The returned value was not '$testvalue' but '$currentvalue'\n");

print "Run completed successfully\n";

?>
-------------------------------

[Salvatore Sanfilippo]

Hello Pete,

thank you a lot for your patch.

I hope that the PHP C bindings will get better and better in the next
weeks to bring a low-latency experience, while at the same time the
pure PHP bindings will get better and better to bring a more stable
and up-to-date with the latest command experience :)

Cheers,
Salvatore

> --
>
> You received this message because you are subscribed to the Google Groups "Redis DB" group.
> To post to this group, send email to redi...@googlegroups.com.
> To unsubscribe from this group, send email to redis-db+u...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/redis-db?hl=.
>
>
>



--
Salvatore 'antirez' Sanfilippo
http://invece.org

"Once you have something that grows faster than education grows,
you’re always going to get a pop culture.", Alan Kay

[Nicolas Favre-Félix]

Pete,

Thanks a lot indeed. This is a side-effect of using log that we hadn't accounted for. 

I have merged your patch and updated the github repository.

Only 2 commands are missing now, /auth/ and /sort/. /sort/ is partially supported, but untested.

I will work on those to make sure we reach 1.0-completeness soon.

Nicolas

2009/11/23 Salvatore Sanfilippo <ant...@gmail.com>