Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails cache broken with Redis::Distributed in 7.0.7 #48938

Closed
jdelStrother opened this issue Aug 14, 2023 · 2 comments
Closed

Rails cache broken with Redis::Distributed in 7.0.7 #48938

jdelStrother opened this issue Aug 14, 2023 · 2 comments

Comments

@jdelStrother
Copy link
Contributor

#48645 adds support for write_multi in collection caches, but this now breaks when using Redis::Distributed as a backend. eg:

      config.cache_store = :redis_cache_store, {
        url: [url1, url2],
        ...
      }
}

I'm seeing MAPPED_MSET cannot be used in Redis::Distributed because the keys involved need to be on the same server or because we cannot guarantee that the operation will be atomic.

redis (5.0.7) lib/redis/distributed.rb in mapped_mset at line 306
activesupport (7.0.7) lib/active_support/cache/redis_cache_store.rb in block (2 levels) in write_multi_entries at line 419
connection_pool (2.4.1) lib/connection_pool.rb in block (2 levels) in with at line 110
connection_pool (2.4.1) lib/connection_pool.rb in handle_interrupt at line 109
connection_pool (2.4.1) lib/connection_pool.rb in block in with at line 109
connection_pool (2.4.1) lib/connection_pool.rb in handle_interrupt at line 106
connection_pool (2.4.1) lib/connection_pool.rb in with at line 106
activesupport (7.0.7) lib/active_support/cache/redis_cache_store.rb in block in write_multi_entries at line 418
activesupport (7.0.7) lib/active_support/cache/redis_cache_store.rb in failsafe at line 466
activesupport (7.0.7) lib/active_support/cache/redis_cache_store.rb in write_multi_entries at line 416
activesupport (7.0.7) lib/active_support/cache.rb in block in write_multi at line 415
activesupport (7.0.7) lib/active_support/cache.rb in block in instrument at line 783
activesupport (7.0.7) lib/active_support/notifications.rb in block in instrument at line 206
activesupport (7.0.7) lib/active_support/notifications/instrumenter.rb in instrument at line 24
activesupport (7.0.7) lib/active_support/notifications.rb in instrument at line 206
activesupport (7.0.7) lib/active_support/cache.rb in instrument at line 783
activesupport (7.0.7) lib/active_support/cache.rb in write_multi at line 410
actionview (7.0.7) lib/action_view/renderer/partial_renderer/collection_caching.rb in fetch_or_cache_partial at line 106
actionview (7.0.7) lib/action_view/renderer/partial_renderer/collection_caching.rb in cache_collection_render at line 43
actionview (7.0.7) lib/action_view/renderer/collection_renderer.rb in block in render_collection at line 161
activesupport (7.0.7) lib/active_support/notifications.rb in block in instrument at line 206
activesupport (7.0.7) lib/active_support/notifications/instrumenter.rb in instrument at line 24
activesupport (7.0.7) lib/active_support/notifications.rb in instrument at line 206
actionview (7.0.7) lib/action_view/renderer/collection_renderer.rb in render_collection at line 147
actionview (7.0.7) lib/action_view/renderer/collection_renderer.rb in render_collection_with_partial at line 119
actionview (7.0.7) lib/action_view/renderer/renderer.rb in render_partial_to_object at line 72
actionview (7.0.7) lib/action_view/renderer/renderer.rb in render_to_object at line 27
actionview (7.0.7) lib/action_view/renderer/renderer.rb in render at line 22
actionview (7.0.7) lib/action_view/helpers/rendering_helper.rb in block in render at line 37
actionview (7.0.7) lib/action_view/base.rb in in_rendering_context at line 270
actionview (7.0.7) lib/action_view/helpers/rendering_helper.rb in render at line 33

@byroot perhaps this could be fixed in Redis::Distributed with something like this (pseudocode) ?

    def mapped_mset(hash, atomic: true)
      raise CannotDistribute, :mapped_mset if atomic
      
       hash.group_by { node_for(_1) }.each { |node, keyvalues|
            node.mapped_mset(keyvalues)
       }
    end

want me to take a stab at fixing it?

@jdelStrother jdelStrother changed the title Rails cache broken with Redis::Distributed in 5.0.7 Rails cache broken with Redis::Distributed in 7.0.7 Aug 14, 2023
@byroot
Copy link
Member

byroot commented Aug 14, 2023

want me to take a stab at fixing it?

Sure, go ahead.

jdelStrother added a commit to jdelStrother/rails that referenced this issue Aug 17, 2023
…ionPool

Also adds some test-coverage for RedisCacheStore backed by Redis::Distributed / ConnectionPool

Fixes rails#48938
@skipkayhil
Copy link
Member

Fixed in #48952

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants