Bug 18342 - Set memcached as 'enabled' by default
Summary: Set memcached as 'enabled' by default
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 18250 20801 21426
  Show dependency treegraph
 
Reported: 2017-03-28 07:03 UTC by Mason James
Modified: 2019-10-14 19:55 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Memcached is now required and enabled by default.
Version(s) released in:


Attachments
Bug 18342: Enable memcached by default for new installs (6.84 KB, patch)
2017-09-27 19:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18342: Enable memcached by default for new installs (6.85 KB, patch)
2018-03-23 18:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18342: Mark Cache::Memcached as mandatory (774 bytes, patch)
2018-03-23 18:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18342: Enable memcached by default for new installs (6.43 KB, patch)
2018-04-20 10:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18342: Mark Cache::Memcached as mandatory (845 bytes, patch)
2018-04-20 10:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18342: Enable memcached by default for new installs (6.97 KB, patch)
2018-04-27 07:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18342: Mark Cache::Memcached as mandatory (920 bytes, patch)
2018-04-27 07:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mason James 2017-03-28 07:03:07 UTC
Let's make memcached 'enabled' by default
Comment 1 Marcel de Rooy 2017-03-28 12:08:14 UTC
And remove the other cache types: Cache::Memory etc.
Comment 2 Jonathan Druart 2017-05-11 14:07:10 UTC
(In reply to Marcel de Rooy from comment #1)
> And remove the other cache types: Cache::Memory etc.

Not Cache::Memory, it should be kept as a fallback way in case memcached is not reachable.
Comment 3 Jacek Ablewicz 2017-05-11 17:59:21 UTC
(In reply to Jonathan Druart from comment #2)
> (In reply to Marcel de Rooy from comment #1)
> > And remove the other cache types: Cache::Memory etc.

IMO at least fastmmap definitely should be removed, it's long overdue.

> 
> Not Cache::Memory, it should be kept as a fallback way in case memcached is
> not reachable

Cache::Memory does't  work all that well as a fallback mechanism.. Under normal circumstances it's just slowing things out (values are stored there, but never retrieved back). And under more unusual circumstances (e.g. if we want to flush L1 cache in daemon scripts periodically) it's pretty much the same - while Cache::Memory supports expiration, currently we don't use it (properly), and the final result is once again 100% waste of CPU cycles, without any gains at all.

I wonder if a better fallback for memcached L2 cache could rather be: no L2 cache, or maybe some kind of dummy L2 cache. It does mater only in case of daemon scripts - if it's not a daemon script etc., Cache::Memory will always be a waste of time anyway.
Comment 4 Jonathan Druart 2017-09-27 19:20:41 UTC
Created attachment 67414 [details] [review]
Bug 18342: Enable memcached by default for new installs

Note that there is no way to create an install without memcached.
As it is now considered as stable, there is no point to not use it.

Test plan:
Create a new Koha install and make sure memcached is enabled by default
Comment 5 Katrin Fischer 2017-10-15 09:06:00 UTC
Can this be tested on kohadevbox?
Comment 6 Jonathan Druart 2017-10-16 13:25:03 UTC
(In reply to Katrin Fischer from comment #5)
> Can this be tested on kohadevbox?

You can test koha-create, yes.
Comment 7 Josef Moravec 2018-03-08 12:07:42 UTC
Shouldn't be Cache::Memcached module marked as required in PerlDependencies then?
Comment 8 Jonathan Druart 2018-03-23 18:04:31 UTC
Created attachment 73216 [details] [review]
Bug 18342: Enable memcached by default for new installs

Note that there is no way to create an install without memcached.
As it is now considered as stable, there is no point to not use it.

Test plan:
Create a new Koha install and make sure memcached is enabled by default
Comment 9 Jonathan Druart 2018-03-23 18:04:35 UTC
Created attachment 73217 [details] [review]
Bug 18342: Mark Cache::Memcached as mandatory
Comment 10 Marcel de Rooy 2018-04-13 10:36:24 UTC
This looks good to me, but not enough time to test it anymore
Comment 11 Kyle M Hall 2018-04-20 10:19:40 UTC
Created attachment 74607 [details] [review]
Bug 18342: Enable memcached by default for new installs

Note that there is no way to create an install without memcached.
As it is now considered as stable, there is no point to not use it.

Test plan:
Create a new Koha install and make sure memcached is enabled by default

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Kyle M Hall 2018-04-20 10:19:47 UTC
Created attachment 74608 [details] [review]
Bug 18342: Mark Cache::Memcached as mandatory

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Julian Maurice 2018-04-20 10:36:10 UTC
Shouldn't memcached be moved from Suggests to Depends in debian/control ?
Comment 14 Tomás Cohen Arazi 2018-04-20 10:50:57 UTC
(In reply to Julian Maurice from comment #13)
> Shouldn't memcached be moved from Suggests to Depends in debian/control ?

That's generated
Comment 15 Marcel de Rooy 2018-04-27 07:36:09 UTC
QA: Looking here
Comment 16 Marcel de Rooy 2018-04-27 07:52:17 UTC
Just noting that running koha-create without --create-db (which probably is only theoretical) tells me: failed to load external entity "/etc/koha/sites/test02/koha-conf.xml"
Comment 17 Marcel de Rooy 2018-04-27 07:52:43 UTC
What about removing Cache::FastMmap ?
Comment 18 Marcel de Rooy 2018-04-27 07:53:35 UTC
Created attachment 74904 [details] [review]
Bug 18342: Enable memcached by default for new installs

Note that there is no way to create an install without memcached.
As it is now considered as stable, there is no point to not use it.

Test plan:
Create a new Koha install and make sure memcached is enabled by default

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Marcel de Rooy 2018-04-27 07:53:40 UTC
Created attachment 74905 [details] [review]
Bug 18342: Mark Cache::Memcached as mandatory

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 20 Jonathan Druart 2018-05-03 13:05:08 UTC
(In reply to Marcel de Rooy from comment #16)
> Just noting that running koha-create without --create-db (which probably is
> only theoretical) tells me: failed to load external entity
> "/etc/koha/sites/test02/koha-conf.xml"

See bug 18983

(In reply to Marcel de Rooy from comment #17)
> What about removing Cache::FastMmap ?

See bug 16067

Thanks for QAing this one!
Comment 21 Jonathan Druart 2018-05-03 16:40:41 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 22 Fridolin Somers 2018-07-30 12:21:57 UTC
I prefer not to push on stable branch 17.11.x, its not a real bug.