If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Use Cache::Memcached::Fast instead of Cache::Memcached

RESOLVED FIXED in Bugzilla 6.0

Status

()

Bugzilla
Installation & Upgrading
--
enhancement
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: gerv, Assigned: Luke Bearl)

Tracking

({relnote})

Bugzilla 6.0
relnote
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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

Updated

2 years ago
Severity: normal → enhancement
(Assignee)

Comment 1

2 years ago
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.
(Assignee)

Comment 2

2 years ago
Created attachment 8641325 [details] [diff] [review]
win32_fast_memcached_fix.patch

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

Updated

2 years ago
Attachment #8641325 - Flags: review?(dylan)

Updated

2 years ago
Assignee: general → lbearl
Status: NEW → ASSIGNED

Updated

2 years ago
Component: Bugzilla-General → Installation & Upgrading
Summary: Investigate use of Cache::Memcached::Fast → Use Cache::Memcached::Fast instead of Cache::Memcached
I would suggest looking into https://metacpan.org/pod/CHI.
(Assignee)

Comment 4

2 years ago
(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 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.
Attachment #8641325 - Flags: review?(dylan) → review+
(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.
Flags: approval?

Updated

2 years ago
Flags: approval? → approval+
Target Milestone: --- → Bugzilla 6.0

Updated

2 years ago
Keywords: relnote

Comment 7

2 years ago
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

2 years ago
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

2 years ago
gerv: agree to require 0.17 based on comment 7? If yes, please fix that on checkin.
Flags: needinfo?(gerv)
(Reporter)

Comment 10

2 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   3db657e..194ed92  master -> master

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

Gerv
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(gerv)
Resolution: --- → FIXED
(Reporter)

Comment 11

2 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   141d0d9..d7b46a6  master -> master

fixes comment 7. Oops.

Gerv
Blocks: 1274764
Blocks: 1308032
You need to log in before you can comment on or make changes to this bug.