https://www.drupal.org/node/2056373 Added basic cache tag support, using the database tag service. This covers most of what we need. There are still issues as we cannot invalidate or delete a whole bin properly yet. Without nuking everything.
A couple of options:
- Similar to 7.x (probably favourable), store a timestamp, probably in state, for the last invalidated or deleted times. Check this when determining if an item(s) is valid. Could apply to both invalidateAll and deleteAll. See below...
- Have a generated prefix that we just regenerate when deleteaAll() is used.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | interdiff-2484595-15.txt | 1.78 KB | damiankloip |
| #15 | 2484595-15.patch | 5.91 KB | damiankloip |
| #14 | interdiff-2484595-14.txt | 671 bytes | damiankloip |
| #14 | 2484595-14.patch | 5.91 KB | damiankloip |
| #12 | interdiff-2484595-12.txt | 3.13 KB | damiankloip |
Comments
Comment #1
mcdruid commentedHere's a very rough patch which implements a randomly generated key pre/suffix per bin, which we then regenerate when
invalidateAll()is called.Comment #2
mcdruid commentedTried a few different ways of using state properly (e.g. injecting it in services) but that seems to lead to "Circular reference" issues.
Here's an alternative approach; using memcache itself to store the key suffixes. Seems quite a bit simpler, and AFAICS it works.
Comment #3
misc commentedSeems like a good approach @mcdruid if no one else have time to review this I try do it the coming week.
Comment #4
webchickTagging this as a beta blocker, per #2989594: [META] Create a stable release of Memcache module for Drupal 8.
Comment #5
webchickAhem.
Comment #6
damiankloip commentedHere is an extended patch for this, the original approach will work for deletions but invalidations, not so much. I.e. requesting an item with $allow_invalid will not work with this approach. So I think we need a way to do prefixing (for deletions) but also invalidations only, with a microtime per bin.
Comment #7
damiankloip commentedAssigning back to critical, this is completely missing functionality from the cache backend.
Comment #8
damiankloip commentedThis approach might be better, using cache tags for invalidations, and using a timestamp for delete all time.
We can then utilise new services for both cache tags and delete timestamp prefixing in the future to improve the stability of this.
Comment #9
fabianx commentedOverall looks almost RTBC.
no longer needed
Let's mark all of that as @internal.
I have not given up hope to use an injected timestamp service instead ;).
Should create the last_bin_deletion time here - though it's really unlikely we would not have one here as we do it on __construct(), so likely not needed.
Are we sure we want to use microtime for this?
Usually timedrift can mean cache corruption, but using normal seconds we just loose some data.
There was a patch in the D7 queue on that to convert to microtime [which gave horrible effects], so I am tending to use normal seconds instead.
Comment #10
fabianx commentedAs discussed we'll continue to use microtime() rounded plus some skew like ChainedFast.
As we are dealing with volatile storage, which can evict keys without notice, we can't rely on atomic increment / decrement behavior unfortunately.
Comment #11
catchThis is the hunk from ChainedFastBackend for reference:
Approach here looks right to me, agreed with tags for invalidations and state for deletions.
Comment #12
damiankloip commentedYes, the skew is a good idea here. We talked about all the other stuff too! Here is a patch which I think is good now.
Comment #13
vurt commented#12 applied fine against the current version.
With PHP 5.6 I get the following error:
When I remove the float it seems to work fine.
A crude performance test with "time drush cr" shows that the new version is about 10% slower than the old one (with global flush).
Thanks for the work! I am happy to see that memcache is moving forward.
Comment #14
damiankloip commentedThanks for testing vurt! Very useful.
It's a good point about the float typehinting. I forget that we still support PHP5.6 sometimes :)
I think the additional time is ok, as before everything was just doing a memcache flush() directly.
Having this functionality in is important, so going to commit this. We can follow up anything related from here.
Comment #15
damiankloip commentedehm ,this.
Comment #17
damiankloip commented