BUG: cache constantly being cleared on Redis / Memcached / LibMemcached / WinCache / APC / XCache

[sunnysideup]

**From what I understand this any site running APC, Libmemcached/Memcached, Redis or WinCache as a cache for Silvertripe: the entire cache gets cleared every time ANY dataobject is written **

This is obviously not workable on any site that regularly writes to the database (userforms, e-commerce, logs, any user logging in and doing stuff)

Here is the evidence:

---

1. we write a data object:

https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/DataObject.php#L1366

2. as the last step of the write function, the cache is flushed

https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/DataObject.php#L1407

3. in the first line, Aggregate::flushCache is called

https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/DataObject.php#L3257

4. in this function, if the back-end does not support "tags" then the whole cache gets cleaned!

https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/Aggregate.php#L51-L65

We are running a site that was clearing the cache every half second or so.

---

caches affected are:

• Libmemcached

• Memcached

• Redis .... I think Redis actually supports tags, but the library I was looking at does not: https://github.com/andrelohmann/silverstripe-cache-backends/blob/master/thirdparty/Zend/Cache/Backend/Redis.php#L381)

• WinCache

• APC

• Xcache
  caches unaffected are:

• files

• sqllite

• mongodb

[tractorcow]

Aggegate is removed in 4.0, so just to clarify this is a 3.x only issue.

[sunnysideup]

Aggegate is removed in 4.0, so just to clarify this is a 3.x only issue.

Yes, but maybe the flushcache method has been moved somewhere else? I think it would make more sense for this to be looked at by core team.

[sunnysideup]

Basically, since 22 Nov 2013, the above caches have been pretty much useless (I can provide more detail if needed). I have fixed this by adding the below in my composer.json script:

        "scripts": {
            "post-install-cmd": [
                "sed -i '/CLEANING_MODE_ALL/d' ./framework/model/Aggregate.php"
            ],
            "post-update-cmd": [
                "sed -i '/CLEANING_MODE_ALL/d' ./framework/model/Aggregate.php"
            ]
        },

It took me a long time to hunt down this bug, therefore, unless no one is using the above caches, I think it is important to fix this bug in SS 3 (from what I understand we can simply remove this line: https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/Aggregate.php#L63).

To give a bit of background: on one of our websites the loading time of the more complex pages went down from 4s to 2.7s (basically the difference between without and with cache) and of course we needed a lot less resources on the server.

[tractorcow]

I wonder if the aggegrate class in 3.x even needs to be cleared that aggressively; I think the cache is already keyed by latest modified date isn't it? Or at least that could be a better solution. :)

[sunnysideup]

I think the cache is already keyed by latest modified date isn't it

That depends on the cache. You can make whatever caches you like and there are also a bunch of core ones:

• LeftAndMain_CMSVersion: Cache the CMS version derived from composer.lock

• CMSMain_SiteTreeHints: Cache the calculation of CMSMain::SiteTreeHints

• GDBackend_Manipulations: Retain the pass/fail state of manipulation calls between executions (this cache is pretty meaningless AFAIK)

• SS_Configuration: Cache the ConfigManifest (not sure how this works)

• i18n: Passed to Zend_Translate

• cacheblock: Cache partial caches

OR are you talking about the aggregate cache specifically? I dont know what it does.

[tractorcow]

The aggregate in particular... the whole issue with this code is that these other caches are cleared due to the aggressiveness of aggregate trying to clear its own cache.

[sunnysideup]

When the cache is saved, no key is being provided, but instead "tags" are used: https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/Aggregate.php#L124-L127

Why not change tags into keys and clear all relevant keys on flushCache
OR
Abandon the caching altogether
OR
only cache if the cache used allows tagging

[sunnysideup]

From what I can tell, Aggregate is not even used in 3.1+... is that right?

https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/Aggregate.php#L32

[sunnysideup]

@chillu @tractorcow , can you confirm this as a bug and if so, will it be fixed and/or will the docs be updated? Should I sent out a warning to the dev list not to use the cache methods above until this has been fixed?

[kinglozzer]

I can’t really see any nice way of fixing this. Aggregate isn’t used in core any more, but that doesn’t mean that other devs aren’t still using it in their own code - changing the behaviour here means we risk breaking things for others.

Perhaps we could have a config flag in DataObject for flush_aggregate_cache_on_write which defaults to true, and add a note to the docs about setting it to false if you’re using a cache backend that doesn’t support tags? It’s hacky, it sucks, but it’s the least intrusive fix that I can spot... open to ideas

[sunnysideup]

Hi Loz,

I like your solution. Lets do it and also update the docs. Would you like me to make the change or would it be better for the core team to do this?

In my world, if you have to choose between:

1. adding a hacky fix to Aggregate

or

2. having your complete caching system (apart from file caching) pretty much useless including misleading docs

then, to me, the choice is an easy one, especially because caching will be more important on bigger,
more complex sites, which is where the SS's worth as a framework is really being tested.

[sunnysideup]

If you like me to make the changes then could you please let me know:

a. what branch I should change exactly

b. is there anything special about the commit message I should know

c. anything else I should think about

I am asking this because I seldom do these core commits and I always seem to screw it up in the first go ;-))

Thank you

Nicolaas

[tractorcow]

I would hacky fix to aggregate. :) Perhaps by fixing caching so it doesn't rely on tags, but instead uses a more distinct key (or named factory) it can safely clear on.

[kinglozzer]

How about this in framework/_config.php:

$cachedir = TEMP_FOLDER . DIRECTORY_SEPARATOR . 'aggregatecache';

if (!is_dir($cachedir)) {
	mkdir($cachedir);
}

SS_Cache::add_backend('aggregatecache', 'File', array('cache_dir' => $cachedir));
SS_Cache::pick_backend('aggregatecache', 'aggregate');

That should ensure that aggregate always uses a filesystem-backed cache, even if the default cache is set to something else. I think this could be acceptable (assuming it actually works of course), given how rare it is that:

1. People change the default cache backend to something other than File
2. Anyone actually uses aggregate at all

Thoughts?

[dhensby]

a. what branch I should change exactly

Assuming this is a bug fix and not a new feature, then 3.4 seems best to me

b. is there anything special about the commit message I should know

Just make it descriptive and with the word "FIX" at the front

c. anything else I should think about

The aim is to make sure you don't break anything others will rely on, that means keeping function parameters unchanged and not changing return types. Obviously a change in behaviour that patches the bug is acceptable.

[chillu]

@sunnysideup Could you please check if that's still an issue for you with the new Symfony/Cache implementation in 4.x?

[dhensby]

@sunnysideup bump

[sunnysideup]

Hi Guys,

I see my role within the Silverstripe platform to (a) create modules (b) make them more accessible - e.g. www.ssmods.com, (c) report bugs.

Unfortunately, I dont really have the time or skill set to fix / test bugs... so I will have to bump it back to you .

Having said that, if this piece of code: https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/Aggregate.php#L51-L65 is gone in 4.0 then I am pretty sure it will work better in 4.0.

[tractorcow]

That entire class is gone @sunnysideup so that has to be a good sign!

[sunnysideup]

It is a good sign.

What I can tell you is that the

		} elseif($capabilities['tags']) {
			$tags = ClassInfo::ancestry($class);
			foreach($tags as &$tag) {
				$tag = preg_replace('/[^a-zA-Z0-9_]/', '_', $tag);
			}
			$cache->clean(Zend_Cache::CLEANING_MODE_MATCHING_ANY_TAG, $tags);
		} else {
			$cache->clean(Zend_Cache::CLEANING_MODE_ALL);
		}

$capabilities['tags'] was not supported by memcache and so the whole cache was cleared on every DB write.

Testing that it actually works now is not trivial because you have to set up a ton of things to be sure it is working. I am just setting up my first 4.0 site now so I am not the best person to help here.

[kinglozzer]

Given that Aggregate is kill, I think we can safely close this. https://github.com/silverstripe/silverstripe-framework/pull/6570/files was also merged, which I believe addressed this issue directly

[patricknelson]

Also worth noting for those of us still crippled with ancient versions of SilverStripe (3.x) that this also only affects you if you define those caches for any or aggregate (as suggested above) and not some specific cache, e.g. cacheblock. For example:

// Bug!
SS_Cache::pick_backend('primary_redis', 'any', 10);

// No bug.
SS_Cache::pick_backend('primary_redis', 'cacheblock', 10);

Simple workaround for those of us who still need to limp along until we can finally move to SS v4.