Hi,
I got some problems will testing this module, I debugged it with phpRedisAdmin and I discovered my cache was stored under several "root keys" : default_, my.frontoffice.hostname, my.backoffice.hostname, my.server.ip, ...
I read again the readme.md and I found nearly at the end (line 201) "Note that if you don't specify the default behavior, the Redis module will attempt to use the HTTP_HOST variable in order to provide a multisite safe default behavior."
I think this information is critical and should be move up and place into each examples :
// Always set a default cache prefix (for multisite see below).
$settings['cache_prefix']['default'] = 'mysite_';
$settings['redis.connection']['interface'] = 'PhpRedis'; // Can be "Predis".
(...)
It could be also confusing for memcached users (as me) since this setting exists and is optional (as for Drupal 7 Redis ?).
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | redis-better-default-prefix.patch | 780 bytes | berdir |
| #7 | redis-2846208-7-prefix-doc.patch | 661 bytes | o'briat |
Comments
Comment #2
berdirThis is the same implementation as the old one for 7.x, I think it now has a better implementation.
That said, I think we can use the same as Drupal core uses as APCU cache prefix for things like the class loader cache. See \Drupal\Core\Site\Settings::getApcuPrefix()
Comment #3
o'briatI missed the D7 doc, I will have a look on APCU.
But in the meantime I still think this default settings is critical and should be placed in the examples.
Comment #4
berdirPatches welcome, but the thing is that fixing the default prefix would mean it's no longer critical/important to set it.
Note that I'm not saying you should use apcu, just re-use that method for our own prefix as well. Will be considerably longer, but it should reliably work for different domains/drush.
Comment #5
o'briatI'll provide a patch for the readme.
You're suggesting that a possible improvement is to use the apcu method for generating a prefix if none provided, hence this settings will be optional ?
About replacing HTTP_HOST prefix by the acpu (or other) method, I want to be sure what cases it should manag, here's my list :
* A single Drupal and Redis : no manual prefix is needed to be set, but a default one could be generated (but it should not change whatever the access method to Drupal is : hostnames, ports, IPs, drush,...)
* Several Drupals or applications share the same Redis server (and db) : the prefix should be set manually, if not the generated prefix should be different from one Drupal to another but still be independent from the access method to each Drupal.
* Multi-site Drupal : the prefix should be set dynamically or, if not the generated prefix should be different from one sub-site to another. On this point I never used Drush on multi-site so I don't know how it works
Comment #6
berdirFor the third option, you need to provide the url to drush so it can figure out the right site.
And yes, those are the variants. And \Drupal\Core\Site\Settings::getApcuPrefix() should work for all of them, with the only downside that it will be pretty long. Which isn't an issue for redis, and if we really want to, we could still hash it to shorten it again.
Comment #7
o'briatHere's my readme patch.
I think a new issue should be open for the default prefix generation ?
Comment #8
kmajzlik commentedJust found this a strange issue with Domain Access, with drush etc...
Comment #9
berdirNo, I don't think we should do a separate issue because setting a better default prefix makes your patch no longer true/neccessary.
All we need to do is this and then drush should work fine.
Comment #10
berdirComment #12
berdirCommitted, worked fine for me in manual testing and tests also still pass but that makes sense as we never run that there anyway.
Comment #13
o'briatI don't know if it comes from my dev machine (win7), but when I use your patch no keys are created on Redis.
I try to debug it, the RedisPrefixTrait::getDefaultPrefix() return a valid hash key, but nothing is write into Redis and no error is return.
Providing a prefix in settings.php fix the problem.
Comment #14
berdirMaybe the key is too long or has problematic characters in it? What happens if you put the same value that is generated into it as explicit prefix?
And if that's also not working, then try making it shorter or remove certain characters from it to figure out what exactly is causing the problem. I tried on multiple environments including locally and platform.sh but all linux.
Comment #15
o'briatForget it, it's my webclient phpRedisAdmin that have a maxkeylen set to 100, if I change it, my keys are displayed correctly.
Always use :
Comment #16
berdirYes, the keys get pretty long, but it's still possible and easy to set a shorter prefix. We could also shorten this again or implement a different version of it, but I actually kind of like that it forces caches to be invalid when you update core or change the deployment identifier.
Setting back to fixed.