|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2010-05-13 17:33 UTC] korchasa at gmail dot com
Description:
------------
'../configure' '--with-fpm' '--with-factcgi' '--with-mcrypt'
'--with-pear' '--enable-pcntl' '--enable-sysvmsg' '--enable-
sysvsem' '--enable-sysvshm' '--with-mysql=mysqlnd' '--with-
mysqli=mysqlnd' '--with-config-file-path=/etc/php.ini' '--
with-config-file-scan-dir=/etc/php.d' '--with-zlib' '--with-
curl' '--enable-mbstring' '--with-libevent=shared'
Memcached version in my case 1.4.2.
Have the same problem at Gentoo, PHP 5.2.13, pecl-memcache
3.0.4, memcached 1.3.3-r2.
Reproduce code:
---------------
<?php
$m = new Memcache();
$m->connect('localhost', 11211);
var_dump($m->increment('a', 42));
var_dump($m->get('a'));
$m->set('b', 'bar');
var_dump($m->increment('b', 42));
var_dump($m->get('b'));
$m->set('c', 1);
var_dump($m->increment('c', 42));
var_dump($m->get('c'));
$m->flush();
Expected result:
----------------
42
42
42
42
43
43
Actual result:
--------------
bool(false)
bool(false)
Notice: Memcache::increment(): Server localhost (tcp 11211)
failed with: CLIENT_ERROR cannot increment or decrement non-
numeric value
(0) in /home/korchasa/Dropbox/www-
work/localhost/small/memcache.php on line 10
bool(false)
bool(false)
bool(false)
bool(false)
PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||
Copyright © 2001-2025 The PHP GroupAll rights reserved. |
Last updated: Fri Nov 07 00:00:02 2025 UTC |
The original report is correct, there is a bug here, but the details aren't exactly correct. After creating a new Memcache object the first block of code: ======= var_dump($m->increment('a', 42)); var_dump($m->get('a')); ======= The response is: ======= bool(false) bool(false) ======= And that is exactly what you should get, so this works correctly. Because trying to increment a key that hasn't been set yet is not valid. The PHP docs already spell out that "Memcache::increment() does not create an item if it doesn't already exist." The second block: ======= $m->set('b', 'bar'); var_dump($m->increment('b', 42)); var_dump($m->get('b')); ======= This close, but doesn't exactly work right. I get: ======= PHP Notice: MemcachePool::increment(): Server localhost (tcp 11211, udp 0) failed with: CLIENT_ERROR cannot increment or decrement non-numeric value (0) in /home/akismet/bin/bugs.php on line 12 NULL bool(false) ======= The set works. The increment on a non-numeric value throws a notice and returns NULL, which is contrary to the PHP docs which state "If item specified by key was not numeric and cannot be converted to a number, it will change its value to value.". Instead of NULL for that call I should have got back 42 and 'bar' over written with 42 in memcached. From what I can see NULL should never be returned by increment, it either should work and return the new numeric value or fail and return FALSE. Where things get interesting is the third block of code. By itself this block runs fine and does the right thing: ======= $m->set('c', 1); var_dump($m->increment('c', 42)); var_dump($m->get('c')); ======= Gives you : ======= int(43) int(43) ======= However, if you run both the second and third block together like this: ======= $m->set('b', 'bar'); var_dump($m->increment('b', 42)); var_dump($m->get('b')); $m->set('c', 1); var_dump($m->increment('c', 42)); var_dump($m->get('c')); ======= I get: ======= PHP Notice: MemcachePool::increment(): Server localhost (tcp 11211, udp 0) failed with: CLIENT_ERROR cannot increment or decrement non-numeric value (0) in /home/akismet/bin/bugs.php on line 15 NULL bool(false) bool(false) bool(false) ======= We get the NULL and false for working on the 'b' key, but then all of the 'c' key operations fail as well. Inspecting the $m object indicates that it still has a connection to the memcached server and no connection errors show up. It seems that the resource gets pushed into some unusable state where all subsequent interactions fail. If you look for NULL responses from increment(), then disconnect and reconnect then future requests work correctly. Increment is not the only way to push a connection into this semi-dead state. It appears that anything that throws a notice about memcached calls will do the trick. Like calling delete($key) instead of delete($key, 0). Instead of becoming unusable these invalid actions should return FALSE and allow future requests to be properly handled.Due to a bug in the older versions of Memcached the value to be incremented, was converted to 0. Thus the statement "If item specified by key was not numeric and cannot be converted to a number, it will change its value to value." in the docs was correct. Now it's not. Memcached will return an error on an attempt to increment a value that is not numeric. On the other hand pecl-memcache disconnects from the server and tries to make a failover when it sees the error and that is why all subsequent requests fail. It seems unreasonable to parse the error and call "set" for the new value if it was non-numeric, so the docs should be changed to reflect that this behavior is no longer valid. My suggestion is to generate only a notice for the client errors and not to do a failover, thus client error conditions won't break the memcached connection. I am posting a small patch for that: *================================================* diff -Naur memcache-3.0.5-orig/memcache_ascii_protocol.c memcache-3.0.5/memcache_ascii_protocol.c --- memcache-3.0.5-orig/memcache_ascii_protocol.c 2010-10-04 00:41:21.000000000 +0300 +++ memcache-3.0.5/memcache_ascii_protocol.c 2011-01-26 23:51:39.000000000 +0200 @@ -84,11 +84,13 @@ } else if ( mmc_str_left(line, "ERROR", line_len, sizeof("ERROR")-1) || - mmc_str_left(line, "SERVER_ERROR", line_len, sizeof("SERVER_ERROR")-1) || - mmc_str_left(line, "CLIENT_ERROR", line_len, sizeof("CLIENT_ERROR")-1)) + mmc_str_left(line, "SERVER_ERROR", line_len, sizeof("SERVER_ERROR")-1)) { response = MMC_RESPONSE_ERROR; } + else if (mmc_str_left(line, "CLIENT_ERROR", line_len, sizeof("CLIENT_ERROR")-1)) { + response = MMC_RESPONSE_CLIENT_ERROR; + } else { response = MMC_RESPONSE_UNKNOWN; } diff -Naur memcache-3.0.5-orig/memcache.c memcache- 3.0.5/memcache.c --- memcache-3.0.5-orig/memcache.c 2010-10-04 00:41:21.000000000 +0300 +++ memcache-3.0.5/memcache.c 2011-01-27 00:29:05.000000000 +0200 @@ -396,7 +396,8 @@ } /* return FALSE or catch memory errors without failover */ - if (response == MMC_RESPONSE_EXISTS || response == MMC_RESPONSE_OUT_OF_MEMORY || response == MMC_RESPONSE_TOO_LARGE) { + if (response == MMC_RESPONSE_EXISTS || response == MMC_RESPONSE_OUT_OF_MEMORY || + response == MMC_RESPONSE_TOO_LARGE || response == MMC_RESPONSE_CLIENT_ERROR) { ZVAL_FALSE(result); if (response != MMC_RESPONSE_EXISTS) { @@ -542,7 +543,7 @@ return MMC_REQUEST_DONE; } - if (response == MMC_RESPONSE_NOT_FOUND) { + if (response == MMC_RESPONSE_NOT_FOUND || response == MMC_RESPONSE_CLIENT_ERROR) { if (Z_TYPE_P(result) == IS_ARRAY) { add_assoc_bool_ex(result, request- >key, request->key_len + 1, 0); } @@ -550,6 +551,11 @@ ZVAL_FALSE(result); } + if (response != MMC_RESPONSE_NOT_FOUND) { + php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Server %s (tcp %d, udp %d) failed with: %s (%d)", + mmc->host, mmc->tcp.port, mmc->udp.port, message, response); + } + return MMC_REQUEST_DONE; } @@ -1918,6 +1924,14 @@ (*((int *)param))++; return MMC_REQUEST_DONE; } + + if (response == MMC_RESPONSE_CLIENT_ERROR) { + ZVAL_FALSE((zval *)param); + php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Server %s (tcp %d, udp %d) failed with: %s (%d)", + mmc->host, mmc->tcp.port, mmc->udp.port, message, response); + + return MMC_REQUEST_DONE; + } return mmc_request_failure(mmc, request->io, message, message_len, 0 TSRMLS_CC); } diff -Naur memcache-3.0.5-orig/memcache_pool.h memcache- 3.0.5/memcache_pool.h --- memcache-3.0.5-orig/memcache_pool.h 2010-10-04 00:41:21.000000000 +0300 +++ memcache-3.0.5/memcache_pool.h 2011-01-26 23:52:22.000000000 +0200 @@ -104,6 +104,7 @@ #define MMC_RESPONSE_EXISTS 0x02 /* same as binary protocol */ #define MMC_RESPONSE_TOO_LARGE 0x03 /* same as binary protocol */ #define MMC_RESPONSE_NOT_STORED 0x05 /* same as binary protocol */ +#define MMC_RESPONSE_CLIENT_ERROR 0x06 #define MMC_RESPONSE_UNKNOWN_CMD 0x81 /* same as binary protocol */ #define MMC_RESPONSE_OUT_OF_MEMORY 0x82 /* same as binary protocol */ diff -Naur memcache-3.0.5-orig/memcache_session.c memcache- 3.0.5/memcache_session.c --- memcache-3.0.5-orig/memcache_session.c 2010-10-04 00:41:21.000000000 +0300 +++ memcache-3.0.5/memcache_session.c 2011-01-27 00:25:50.000000000 +0200 @@ -426,6 +426,14 @@ return MMC_REQUEST_DONE; } + if (response == MMC_RESPONSE_CLIENT_ERROR) { + ZVAL_FALSE((zval *)param); + php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Server %s (tcp %d, udp %d) failed with: %s (%d)", + mmc->host, mmc->tcp.port, mmc->udp.port, message, response); + + return MMC_REQUEST_DONE; + } + return mmc_request_failure(mmc, request->io, message, message_len, 0 TSRMLS_CC); } /* }}} */ *================================================*