Cache bin handling in multiple Redis DBs

Created on 18 December 2014, over 10 years ago
Updated 8 June 2023, over 2 years ago

Perhaps it would be nice to handle different cache bins in multiple Redis DBs.

Why? Currently Redis doesn't support wildcard delete so the Redis module have to clear all keys one by one for a cache clear of one cache bin. This could be possible failed with an PHP execution timeout, when you Redis Cache have many many keys. With a seperate Redis DB for one cache bin the module could use a flushdb to clear a hole cache bin.

Problem: How to configure this and are their any performance issues, when you have every time switch between the different Redis DBs?

Perhaps someone have any ideas on that. We have tried a patch with a LUA script handling for a wildcard delete, but then our Redis Server sometimes could fail after a flush all caches because it is also busy from the LUA script. Patch can be found here #2140897-43: cache_clear_all() not properly handled in PhpRedis.php

Feature request
Status

Needs review

Version

1.0

Component

Code

Created by

🇩🇪Germany IT-Cru Munich

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇬🇧United Kingdom somersoft

    This patch enables the ability to use a multiple Redis services and configure which cache bin use which service based upon the cache bin name.
    The Redis report has been greatly modified to report information per service. It also lists which cache bins have been configured to use each service.
    The patch has been used with AWS services in production.
    It is fully backward compatible. for single service use and there are no changes to limit the configuration options.
    Documentation has also been updated to indicate how to add and configure multiple services.

  • 🇬🇧United Kingdom somersoft

    Updated the patch for redis 1.7
    Here is the list of patches from composer.patches.json

        "drupal/redis": {
          "Currently Drush Cr or Cache Clear UI does not flush Redis cache - https://www.drupal.org/project/redis/issues/2765895": "https://www.drupal.org/files/issues/redis-n2765895-16.patch",
          "Support TLS for Predis - https://www.drupal.org/project/redis/issues/3004561": "https://www.drupal.org/files/issues/2021-11-19/redis-support_tls_on_predis-3004561-37.patch",
          "Implement initial RedisCluster client integration - https://www.drupal.org/project/redis/issues/2900947": "https://www.drupal.org/files/issues/2024-01-15/2900947-60.patch",
          "Cache bin handling in multiple Redis DBs - https://www.drupal.org/project/redis/issues/2395255": "https://www.drupal.org/files/issues/2024-02-01/redis-Cache_bin_handling_in_multiple_Redis_DBs-2395255-5.patch"
        },
    
  • 🇬🇧United Kingdom somersoft

    Updated the patch for redis 1.8
    Here is the list of patches from composer[.patches].json

        "drupal/redis": {
          "Currently Drush Cr or Cache Clear UI does not flush Redis cache - https://www.drupal.org/project/redis/issues/2765895": "https://www.drupal.org/files/issues/redis-n2765895-16.patch",
          "Support TLS for Predis - https://www.drupal.org/project/redis/issues/3004561": "https://www.drupal.org/files/issues/2021-11-19/redis-support_tls_on_predis-3004561-37.patch",
          "Implement initial RedisCluster client integration - https://www.drupal.org/project/redis/issues/2900947": "https://www.drupal.org/files/issues/2024-10-15/2900947-82.patch",
          "Cache bin handling in multiple Redis DBs - https://www.drupal.org/project/redis/issues/2395255": "https://www.drupal.org/files/issues/2024-10-27/redis-Cache_bin_handling_in_multiple_Redis_DBs-2395255-6.patch"
        },
    
  • 🇳🇱Netherlands casey

    Nice. This is also useful if you want things like queues and persistent locks to be stored in redis instances with persistence enabled, while caches can be stored in redis instances without persistence.

    Note that I don't really like the name $settings['redis_client_bins']. I would go for $settings['redis.connections'] (or redis.servers) and $settings['redis.targets'] (or redis.bindings) and have the redis CacheBackendFactory look for "cache:$bin" and "cache". Maybe the same can be useful for queues.

    Also, the patch seems to create a client per cache bin, even if they are configured to use the same server. I guess this can be done more efficiently.

  • 🇬🇧United Kingdom somersoft

    @casey
    The index redis.connection is already used. It is for connection details when there is only one and acts the fallback conection configuration for when no further server is configured for that cache bin. The change does not break any current configurations.
    FYI:
    cache bin is mentioned here core/lib/Drupal/Core/Cache/CacheFactory.php::24
    'bins' and 'servers' are the indexes used in the Memcache configuration. It seems they had the functionality fromearlier on. Wanted the configuration concepts for both to be very similar so moving between using different techonlogies was not difficult for evalution purposes.
    Feel free to make updates to the patch file and create one client per server rather than per bin by updating
    Drupal\redis\ClientFactory::getClientInterface() and/or ::getClient()

  • 🇬🇧United Kingdom somersoft

    The changes since #6

    • I have taken onboard the comment about one client per cache bin and now it is one client per service endpoint.
    • Some of the calls to getClient() did not pass a parameter where the context provide one.
    • The _container_ cache was added to the report
    • The report was tweaked for reporting locks, flood, queue etc entries.
    • The name of the settings key is now a class constant so that if it needs to change, it is only changed in one place.

    Notes

    • The patch is applied against the 1.9.0 tag.
    • The list of other patches applied before this remains the same.
  • 🇬🇧United Kingdom somersoft

    The changes since #10

  • Looked at the code brought in by the patch from Currently Drush Cr or Cache Clear UI does not flush Redis cache 🐛 Currently Drush Cr or Cache Clear UI does not flush Redis cache Fixed and found it not to work as expected.
    • In the multiple service endpoints some of the clients might not be loaded at the time of calling.
    • The redis connections configuration work if the 'base' is not defined.
    • Delete keys other than those used for cache functionality.

    I acknowledge the work implemented by those developers that made the change and it helped this change.

  • I have added a wrapper class around RedisPrefixTrait so that the hook_cache_flush() can use the functionality. At this moment it does not seem that the bin specific prefix are used only the default prefix. The notes in README.md have been updated to so that are possibly more informative.
  • the Information in the Report has increased to
    • show which bins will be cleared by resetting the cache
    • the default prefix is now displayed
    • the prefix settings are now displayed
    • the connection settings are now displayed
    • Removed deprecated functions
  • There is now a separate constant for the default _BIN_ name
  • Notes

    • The patch is applied against the 1.9.0 tag.
    • Maintain compatibility when using a single service. The extra information on in the report could be helpful.
    • The changed files now produce less PHPCS Errors and Warnings.
    • The list of other patches applied before this has changed.
      composer.[patches.].json
          "drupal/redis": {
            "Support TLS for Predis - https://www.drupal.org/project/redis/issues/3004561": "https://git.drupalcode.org/project/redis/-/merge_requests/1.diff",
            "Implement initial RedisCluster client integration - https://www.drupal.org/project/redis/issues/2900947": "https://www.drupal.org/files/issues/2024-10-15/2900947-82.patch",
            "Cache bin handling in multiple Redis DBs - https://www.drupal.org/project/redis/issues/2395255": "https://www.drupal.org/files/issues/2025-04-27/redis-Cache_bin_handling_in_multiple_Redis_DBs-2395255-11.patch"
          },
      
    • Add a comment Flood Contol issue about recommended changes to use this functionality.
  • 🇬🇧United Kingdom somersoft

    The changes since #11

    • Fixed the missing command display for slow logs
    • Changed the sorting of bins so those with the same number keys are now not randomly ordered
    • Replaced many fixed strings with the use of class constants
  • 🇬🇧United Kingdom somersoft

    Notes

    • The patch is applied against the 1.10.0 tag.
    • The list of other patches applied before this has changed. Dropped RedisCluster client integration Implement initial RedisCluster client integration Needs review
      composer.[patches.].json
       "drupal/redis": {
            "Support TLS for Predis - https://www.drupal.org/project/redis/issues/3004561": "https://git.drupalcode.org/project/redis/-/merge_requests/1.diff",
            "Cache bin handling in multiple Redis DBs - https://www.drupal.org/project/redis/issues/2395255": "https://www.drupal.org/files/issues/2025-04-27/redis-Cache_bin_handling_in_multiple_Redis_DBs-2395255-13.patch"
          }

    Interdiff Was created using diff as interdiff generated an error.
    Used these instructions Misleading error message when uploading a file Needs work .

  • 🇨🇭Switzerland berdir Switzerland
          "Support TLS for Predis - https://www.drupal.org/project/redis/issues/3004561": "https://git.drupalcode.org/project/redis/-/merge_requests/1.diff",
    

    .

    Do not EVER use merge requests in composer patches like this. Anyone change this code. this is a remote code execution vulnerability. save the patch files locally.

  • 🇬🇧United Kingdom somersoft

    @berdir. Okay, Those that want to use it will needs to increase the management overhead of saving the diff file each time the code is updated merge request is updated but the vulnerability injection delivery route does not go away when you do that as it could be injected with pushes to the repository and/or uploaded patches attached to comments.

    1. In general with using patches it is recommended that all code developers examine and regularly re-examine all patches no matter how they are sourced.
    2. Why is the model of an issue repository and merge request indicated to be the way forward, if this risk is so great? Perhaps the maintainers of project/packages should feedback to the policy makers?
    3. As a suggestion perhaps project maintainers should indicated the project's preference for either issues to use or not use the issue repositories. It does give the advantage to the developer and maintainers that the CI can be run and it reporting on code quality and single button push to merge the changes in for the maintainers. Or perhaps that after what seems to be the final push a patch file is uploaded so that the best of both worlds.
    4. The time spent by developers on the management of patches from long time running issues, across new releases, can be more than hoped for..

    I will produce a patch for #3004561 Support TLS for Predis Needs review based upon the MR.

    The information is present for others developers as probably the MR/patch will not apply with out it.

    This and other issues are to improve the generic functionality of the project in a fast changing world.

  • 🇬🇧United Kingdom somersoft

    Requested patch created. See this comment Support TLS for Predis Needs review .

  • 🇨🇭Switzerland berdir Switzerland

    > but the vulnerability injection delivery route does not go away

    *everyone* can get access to the merge request, change it and then next time you run composer install you will immediately get the latest version, without the ability to review or see those changes. A local patch file (which you should use, not post it in the issue) or a URL to an uploaded file requires an explicit action by someone and it gives you either diff for the local file or some context on who uploaded that file and if you trust that.

    It's not the same, it really isn't.

  • 🇬🇧United Kingdom somersoft

    The changes since #13

    1. Tighten update on the generation of the string passed to scan. It was missing the delimiter so if a prefix was shorten version of ones already in use in the store, then those would be returned as well.
    2. Cope if call to get the config for 'maxmemory*' returns FALSE. This was found when using AWS Valkey endpoint service.

    File SHA512

    • 3d101e14c859548dec86c7cbd11d33c95f66aae98e3ce793d2dbf467284031a03eb4cd6a1d140516f4065592c52359838bd51d1a52382612cc0882f1251a7300 interdiff-2395255-13_18.txt
    • 6a6cd028bf1a2c065bbe1c34197c30b3295338594e50ec8f341651024cc7614e6b844bc2ff7dc17e9de4f5958acfdb47ad42e4b7fb5c1195cbf1cae81b98e60e redis-Cache_bin_handling_in_multiple_Redis_DBs-2395255-18.patch
  • 🇨🇭Switzerland berdir Switzerland

    Development on Redis 2.x has officially started. New features to 8.x-1.x will no longer be considered. This will need to be redone on top of the 2.x branch.

    See 📌 Introduce Adapters instead of different cache/queue/lock classes per backend Active and https://www.drupal.org/node/3541244 . This will need to be completely redone. What I'm thinking about is being able to pass an identifier to the new getClient() method, something namespaced like cache.$bin, and then the factory will check for settings for cache.bin, cache and the default location. That should fastly simplify this, but there are still some challenges such as the reports controller.

    Patches is no longer a supported contribution mechanism, tests only run on merge requests.

  • 🇬🇧United Kingdom somersoft

    The changes since #18

    1. For deleting the cache entries it was found to be better to use the 'cache.backend.redis' service rather than call DEL using the connection directly. Any upstream processing of that functionality will then take place. Updating of the work for 🐛 Currently Drush Cr or Cache Clear UI does not flush Redis cache Fixed
    2. Report the Valkey version if the information is returned under the information 'Server' collection.
    3. it was found that when using Valkey, the 'redis_mode' information was not returned but 'server_mode' was. Display that information

    SHA512sum of uploaded files

    5a8d27838832fbeadbe185084647bc7666cd5cd0c9a8a605e47e5c7945ec0fa6a01a851f2a53300219a1d2e58fd54a579fbe57b84662d7bb633237eccd5a4fc9 interdiff-2395255-18_20.txt
    cd8e88112ef206e798251c53c70ebc8834c1c16ee99aa8914cc8368178ba029595a47d654fc36d28a031680471f4be42715c5256fd68b7bbffa3ff49aab856ed redis-Cache_bin_handling_in_multiple_Redis_DBs-2395255-20.patch

    This work already passes a parameter to getClient() and uses that to determine which connection to use, If the passed value is not configured to use a different connection than the current default connection, then it uses that one. This make the changes backward compatible with current practice. Currently the passed value is $bin as it is already available.

    Currently for Flood, Lock, Queue, Persistent Queue, cacketag and container I was thinking about using the prefix redis. where cache. is currently being returned from \Drupal::getContainer()->getParameter('cache_bins') and possibly displaying this information as well as or instead of the bin data namespace in the Report Controller. It would then possibly make sense to change the association of data namespace from bin to this extended information in the additional connection configuration. If this was done then possibly RedisPrefixTrait and it's documentation and use could be updated to reflect this.

    These changes do already include the Report Controller, which has been extended to display the per connection information as well as other Redis configuration information. There are other changes found when using it with AWS Valkey Elasticache and information on which data namespaces will be cleared as a part of the reworking of #2765895 feature request. The key count is display all data namespaces. A large part of the size of the patch are the changes to Report controller.

    Notes

    • As there is current no released version of 2.x work on the 1.x continues.
    • All the changes are backward compatible with current configuration structures. Users just might want to use the patch to see the changes to Report Controller and have #2765895 functionality.
    • At this moment the use of compression is not configurable per connection. Something to discuss with users of this module?
  • Production build 0.71.5 2024