It would seem that unlike the Memcache PECL module, the Memcached module doesn't take options from php.ini. Instead you use setOption on the memcache object in PHP. This means that anyone using the Memcached module (and since we check for that one first, that maybe people who don't mean to) are using all the memcache defaults (including modA distribution). Attached is a quick patch to add a memcache_options array to settings.php, allowing you to set options for the memcache object.

Example config would be:

$conf['memcache_options'] = array(
Memcached::OPT_COMPRESSION => false,
Memcached::OPT_BINARY_PROTOCOL => true,
);

CommentFileSizeAuthor
#13 854516-memcache-options-d7.patch2.64 KBrobertdouglass
#12 854516-memcache-options.patch2.66 KBlongwave
#8 memcache-options.diff2.78 KBrobertdouglass
#4 memcache-options.diff2.75 KBnnewton
#2 memcache-options.diff820 bytesnnewton

Comments

jeremy’s picture

jeremy commented
Status: Active » Needs work

There is no patch attached.

nnewton’s picture

nnewton commented
Status: Needs work » Needs review
StatusFileSize
new memcache-options.diff820 bytes

Thats odd, thought I'd attached it...here it is.

jeremy’s picture

jeremy commented
Status: Needs review » Needs work

Let's set sane defaults:
http://www.php.net/manual/en/memcached.constants.php

Please update this issue with details on why our defaults are chosen.

nnewton’s picture

nnewton commented
Status: Needs work » Needs review
StatusFileSize
new memcache-options.diff2.75 KB

Attaching a patch with sane defaults and documentation.

jeremy’s picture

jeremy commented
Status: Needs review » Patch (to be ported)

Thanks, patch committed in 7.x-1.x-dev. Still needs to be ported to 6.x-1.x-dev.

nnewton’s picture

nnewton commented

Note, the memcached module has a significant performance regression it its current form...this patch is fine, but the PECL module its enabling may not be.

jeremy’s picture

jeremy commented
Version: 7.x-1.x-dev » 6.x-1.x-dev

Re-assigning to 6.x, where we need the port.

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new memcache-options.diff2.78 KB

Here's the rerolled patch. The D7 patch applied with fuzz.

robertdouglass’s picture

robertdouglass commented
Status: Patch (to be ported) » Fixed
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Fixed » Active

Not sure these defaults are entirely sane, as binary protocol is only supported in memcached 1.4 and Debian stable currently ships memcached 1.2.2, which meant the default configuration gave WSOD on all pages on my server until I figured out what was going on. Or at least we should document that OPT_BINARY_PROTOCOL should be false for older versions of memcached?

robertdouglass’s picture

robertdouglass commented

@longwave thanks for testing. Can you propose a fix or a documentation patch (maybe mentioning your platform and experience?)

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 854516-memcache-options.patch2.66 KB

Proposed fix attached. This switches off the binary protocol by default, as the default configuration should then work with all versions of memcached, but documents the fact it should be enabled if memcached 1.4 or above is used.

The patch also sets the default options before overriding with any changes from settings.php (rather than having to copy the defaults into settings.php, as in the previous code), conforms better to Drupal coding standards (FALSE instead of false) and fixes a minor spelling error in the docs.

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new 854516-memcache-options-d7.patch2.64 KB

D7 version of the patch.

markpavlitski’s picture

markpavlitski commented
Status: Needs review » Fixed

Looks like this is fixed in latest 6.x-1.x and 7.x-1.x.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.