Bug 17188

Summary:	Koha does not support several memcached servers
Product:	Koha	Reporter:	Jonathan Druart <jonathan.druart>
Component:	Architecture, internals, and plumbing	Assignee:	Jonathan Druart <jonathan.druart>
Status:	CLOSED INVALID	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	abl
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Bug Depends on:
Bug Blocks:	11921, 16579
Attachments:	Bug 17188: Fix support of several memcached servers

Description Jonathan Druart 2016-08-24 11:32:41 UTC

There is a bug in the initialisation of memcached, the server string is
not correctly parsed.
If several memcached servers are defined, they are separated by commas
and so the string should be split accordingly.

Comment 1 Jonathan Druart 2016-08-24 11:33:41 UTC

Created attachment 54814 [details] [review]
Bug 17188: Fix support of several memcached servers

There is a bug in the initialisation of memcached, the server string is
not correctly parsed.
If several memcached servers are defined, they are separated by commas
and so the string should be split accordingly

Test plan:
1/ Set MEMCACHED_SERVERS='localhost:11211,localhost:11222'
2/ Set DEBUG=1
3/ Reload plack, and check the log
=> Without this patch, you see
  Memcached server settings: localhost:11211,localhost:11222 with koha
  Selected caching system: Cache::Memory=HASH(0x9de2034)
i.e. Memcached has not been configured correctly and the default caching
system is used (Cache::Memory)
=> With this patch, you should see
Memcached server settings: localhost:11211, localhost:11222 with koha
Selected caching system: Cache::Memcached::Fast=SCALAR(0xa2a0c54)
i.e. Memcached has been configured correctly \o/

Note that the cache_servers attribute is never set and it not used. It's
better to remove it.

Comment 2 Jacek Ablewicz 2016-09-02 11:48:39 UTC

It's strange, but I'm not able to reproduce this issue in unpatched master (wheezy, 5.14.2, no plack). I'm wondering if plack / no plack can be a factor here (and if yes - why?).

This statement

    my @servers =
      split /,/, $self->{'cache_servers'}
      ? $self->{'cache_servers'}
      : ($ENV{MEMCACHED_SERVERS} || '');

(while ugly as bat s**t) seems to work fine for me, I'm getting:

    Default caching system: memcached at /home/koha/devkohaclone/C4/Context.pm line 514
    Memcached server settings: localhost:11301, localhost:11302 with koha at /home/koha/devkohaclone/C4/Context.pm line 514
    Selected caching system: Cache::Memcached::Fast=SCALAR(0x297b488) at 
/home/koha/devkohaclone/C4/Context.pm line 514

in both patched and unpatched master.

Comment 3 Jonathan Druart 2016-09-05 10:12:57 UTC

Indeed I misread the code and it works as expected.
It's weird I have no idea how I have invented this bug neither this test plan...