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

Various INI deviations or bugs in 3.0.3 #351

Closed
ghGerry opened this issue Jun 7, 2017 · 4 comments
Closed

Various INI deviations or bugs in 3.0.3 #351

ghGerry opened this issue Jun 7, 2017 · 4 comments
Milestone

Comments

@ghGerry
Copy link

ghGerry commented Jun 7, 2017

I've compiled PHP 7.1.5 from source (Debian 8 x64) and added memcached-3.0.3 via PECL. To get a complete overview (because the PHP doc seem to be outdated), I've checked the tarball, found the memcached.ini and copied the settings to my php.ini. (Note: Same problems for adding 3.0.3 to PHP 7.0.19.)

(A) v3.0.3 seems to have problems with parsing INI option "Off" or "off" (as used in the INI) like memcached.sess_remove_failed_servers = Off. Without the option, phpinfo() displays a default value of 0 (which also matches php_memcached.c), and On and also 1 work as expected and display "1" in phpinfo(), but an Off results in a "no value" for local and master column. The Options are marked as OnUpdateBool, but this does not seem to work with "Off".

(B) memcached.sess_randomize_replica_read seems to use the wrong type. According to the line

MEMC_SESSION_INI_ENTRY("randomize_replica_read", "0", OnUpdateLongGEZero, randomize_replica_read_enabled)

I am no professional, but according to the name, it still seems to be a boolean and should use OnUpdateBool (as it does in v2.2.0), shouldn't it? Because of the current setting, I could also successfully assign the value 14, which doesn't make sense, until the meaning has changed and is undocumented, so far. Of course, assigning Off therefore also results in a "no value".

(C) The documentation for memcached.sess_connect_timeout could be clarified in the INI file. I found the following lines in the code:

MEMC_SESSION_INI_ENTRY("connect_timeout", "0", OnUpdateLongGEZero, connect_timeout)

php_memcached_globals->session.connect_timeout = 1000;

For memcached.default_connect_timeout the INI file states, that with -1 sets an infinite timeout and with 0 it uses the memcached internal default value for a timeout (whatever this might be!?).
Could it be a similar behaviour for memcached.sess_connect_timeout? As mentioned, I'm no pro, in phpinfo() "0" is displayed by default and not the "1000".

(D) The documentation for memcached.sess_lock_expire could be clarified in the INI file. The default value 0 refers to the default behaviour: use memcached.sess_lock_max_wait, if that's also 0, use max_execution_time. I found:

/* Deprecated */
STD_PHP_INI_ENTRY("memcached.sess_lock_wait", "not set", PHP_INI_ALL, OnUpdateDeprecatedLockValue, no_effect, zend_php_memcached_globals, php_memcached_globals)
STD_PHP_INI_ENTRY("memcached.sess_lock_max_wait", "not set", PHP_INI_ALL, OnUpdateDeprecatedLockValue, no_effect, zend_php_memcached_globals, php_memcached_globals)

So the referred var is no write error ("max_wait" vs. "wait_max"), but it seems to be a deprecated var. I guess the assigned "not set" should evaluate to/be equal to 0 (?), therefore in the default for memcached.sess_lock_expire is always euqal to the script max_execution_time?

(E) Deviation for INI doc and code for memcached.sess_prefix, als already noted by Zerivael: INI states "memc.sess.key." whereas code by default only uses "memc.sess."

Best regards,

@sodabrew sodabrew added this to the 3.0.4 milestone Jun 13, 2017
@sodabrew
Copy link
Contributor

Thank you for the thorough problem report!

@sodabrew sodabrew modified the milestones: 3.0.4, 3.0.5 Nov 20, 2017
@sodabrew
Copy link
Contributor

sodabrew commented Jan 22, 2018

B resolved by 2fc7d36

E resolved by #340 in 5edd5ba

@sodabrew
Copy link
Contributor

sodabrew commented Jan 23, 2018

A is a display bug. Boolean INI values should use STD_PHP_INI_BOOLEAN instead of STD_PHP_INI_ENTRY macro. The difference is that STD_PHP_INI_BOOLEAN sets an ini value display formatter that interprets the boolean value to On / Off in phpinfo() output. The INI value is parsed correctly, you just can't see it. Fixed in f54ed29.

C the default value is never 1000, because the INI parser runs immediately after those hardcoded defaults, and sets the value to 0. The libmemcached library also accepts -1 to mean wait forever but OnUpdateLongGEZero prevents this value. Fixed in 2c693c7

@sodabrew
Copy link
Contributor

D text fixed in dfcbc02 the rest of the issues around locking delays are discussed in depth in #269

@sodabrew sodabrew modified the milestones: 3.0.5, 3.1.0 Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants