1189281 - Use Cache::Memcached::Fast instead of Cache::Memcached

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, enhancement)

Description

[Gervase Markham [:gerv]]

Bugzilla supports Memcached via Cache::Memcached, which is written in Perl. However, there's a Cache::Memcached::Fast which is written in C and therefore, presumably faster. It also apparently supports Windows. We should investigate whether we should switch to either support both, or just support ::Fast.

Gerv

Comment 1

[Luke Bearl]

I have modified the Bugzilla code in use by my employer in order to use Cache::Memcahced::Fast and so far it is working perfectly on Win32 (but I can't speak for its compatibility with other platforms). I did this due to the fact that the standard Cache::Memcached that Bugzilla attempts to install would fail various tests when being installed, and even after installing with skipping the tests was non-operational.  Just for fun I decided to try out ::Fast since its documentation indicated that it is mostly API compatible with Cache::Memcached, and it appears that the functions bugzilla uses are completely compatible.

Let me know if there are any specific questions.

Comment 2

[Luke Bearl]

Attachment: win32_fast_memcached_fix.patch

Adding the git-patch I created if anyone wants to quickly make the same changes I did.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

I would suggest looking into https://metacpan.org/pod/CHI.

Comment 4

[Luke Bearl]

(In reply to Dylan William Hardison [:dylan] from comment #3)
> I would suggest looking into https://metacpan.org/pod/CHI.

Even if using CHI, the decision would need to be made between the three memcached drivers it supports: Cache::Memcached, Cache::Memcached::Fast, and Cache::Memcached::libmemcached. 

Cache::Memcached can be disqualified immediately if Windows support is needed, ::Fast is known to work with Windows and from looking at the CPAN tester report it works across most other platforms bugzilla supports.  Cache::Memcached::libmemcached appears to be non-operational in CHI currently (see: https://rt.cpan.org/Public/Bug/Display.html?id=89697). 

This brings us full circle back to whether or not to use ::Fast, although with the suggestion of CHI, ::Fast would still need to be used, it would just be abstracted away.

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8641325 [details] [diff] [review]
win32_fast_memcached_fix.patch

Review of attachment 8641325 [details] [diff] [review]:
-----------------------------------------------------------------

r=dylan 

this code works, but it means we depend on an XS module (which is sometimes more trouble). But as a reviewer is only reviewing the content of the code
and not the directionality of the project, this looks fine to me.

Comment 6

[Dylan Hardison [:dylan] (he/him)]

(In reply to Luke Bearl from comment #4)
> (In reply to Dylan William Hardison [:dylan] from comment #3)
> > I would suggest looking into https://metacpan.org/pod/CHI.
> This brings us full circle back to whether or not to use ::Fast, although
> with the suggestion of CHI, ::Fast would still need to be used, it would
> just be abstracted away.

CHI can also let you plug in Cache::FastMmap -- which is quite fast and would be a useful direction to go in for non-CGI bugzillas.

Comment 7

[Frédéric Buclin]

The changelog says that you need at least version 0.17 (release date: 2009-11-02) to work with memcached 1.3.2 or newer. Current release of memcached is 1.4.24. I would suggest that the min version be bumped to 0.17 to avoid incompatibilities.

Comment 8

[Frédéric Buclin]

Did you consider that this module doesn't preserve tainting?

http://search.cpan.org/~kroki/Cache-Memcached-Fast-0.23/lib/Cache/Memcached/Fast.pm#Tainted_data

Comment 9

[Frédéric Buclin]

gerv: agree to require 0.17 based on comment 7? If yes, please fix that on checkin.

Comment 10

[Gervase Markham [:gerv]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   3db657e..194ed92  master -> master

Luke: thanks so much for the patch. Your contributions are appreciated :-)

Gerv

Comment 11

[Gervase Markham [:gerv]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   141d0d9..d7b46a6  master -> master

fixes comment 7. Oops.

Gerv