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

[close #33907] Error when using "recyclable" cache keys with a store that does not support it #33932

Merged
merged 2 commits into from Sep 21, 2018

Conversation

@schneems
Copy link
Member

@schneems schneems commented Sep 20, 2018

If you are using the "in cache versioning" also known as "recyclable cache keys" the cache store must be aware of this scheme, otherwise you will generate cache entries that never invalidate.

This PR adds a check to the initialization process to ensure that if recyclable cache keys are being used via

config.active_record.cache_versioning = true

Then the cache store needs to show that it supports this versioning scheme. Cache stores can let Rails know that they support this scheme by adding a method supports_in_cache_versioning? and returning true.

@schneems schneems force-pushed the schneems:schneems/recyclable-key-support-cache branch from 3e0d702 to 6768c37 Sep 20, 2018
initializer "Check for cache versioning support" do
config.after_initialize do |app|
ActiveSupport.on_load(:active_record) do
if app.config.active_record.cache_versioning && defined?(Rails) && Rails.cache

This comment has been minimized.

@rafaelfranca

rafaelfranca Sep 20, 2018
Member

I think Rails is defined at this point.

This comment has been minimized.

@schneems

schneems Sep 20, 2018
Author Member

good point, it's in a..railtie. Fixed.

…that does not support it

If you are using the "in cache versioning" also known as "recyclable cache keys" the cache store must be aware of this scheme, otherwise you will generate cache entries that never invalidate.

This PR adds a check to the initialization process to ensure that if recyclable cache keys are being used via 

```
config.active_record.cache_versioning = true
```

Then the cache store needs to show that it supports this versioning scheme. Cache stores can let Rails know that they support this scheme by adding a method `supports_in_cache_versioning?` and returning true.
@schneems schneems force-pushed the schneems:schneems/recyclable-key-support-cache branch from 6768c37 to 135d3e1 Sep 20, 2018
You're using a cache store `#{Rails.cache.class}` that does not support
"recyclable" cache keys, also known as "in cache versioning". To
fix this issue either disable "recyclable" cache keys by setting:

This comment has been minimized.

@jeremy

jeremy Sep 20, 2018
Member

Let's stick with cache versioning terminology as a conceptual anchor and rely on the guides to discuss in detail. We can drop quoted terms. Let's be clear about what we're asking of the cache store, too, e.g.

You're using a cache store that doesn't support native cache versioning. Your best option is to upgrade to a newer version of #{Rails.cache.class} that supports cache versioning (#{Rails.cache.class}.supports_cache_versioning? #=> true). Next best, switch to a different cache store that does support cache versioning: https://guides.rubyonrails.org/caching_with_rails.html#cache-stores. To keep using the current cache store, you can turn off cache versioning entirely: config.active_record.cache_versioning = false.

config.after_initialize do |app|
ActiveSupport.on_load(:active_record) do
if app.config.active_record.cache_versioning && Rails.cache
unless Rails.cache.try(:supports_in_cache_versioning?)

This comment has been minimized.

@jeremy

jeremy Sep 20, 2018
Member

Let's use a class method and normalize naming: Rails.cache.class.supports_cache_versioning?

This comment has been minimized.

@schneems

schneems Sep 20, 2018
Author Member

I made a comment about the naming in basecamp. I think it's confusing. All caches are versioned already via cache key, the conceptual change is that the versioning is now in the cache which is what i'm trying to convey here with that word. I can standardize on this though.

def supports_in_cache_versioning?
true
end

This comment has been minimized.

@jeremy

jeremy Sep 20, 2018
Member

Let's do a supports_cache_versioning? class method for these.

Comment may be unwrapped to # Advertise cache versioning support.

(Worth noting that this duck typing won't cover subclasses of these cache stores.)

@@ -1,3 +1,8 @@
* Raise an error when "recyclable cache keys" are being used by a cache store
that does not explicitly support it.

This comment has been minimized.

@jeremy

jeremy Sep 20, 2018
Member

Let's describe what cache stores need to do (implement supports_cache_versioning class method) to advertise support.

This comment has been minimized.

@schneems

schneems Sep 20, 2018
Author Member

For this what are you thinking? Maybe a section in the guide?

- Moving the `supports_cache_versioning?` check to a class method. 
- Shorten the method doc. 
- Expand on the error message.
@schneems schneems merged commit b122345 into rails:master Sep 21, 2018
2 checks passed
2 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
rafaelfranca added a commit that referenced this pull request Sep 21, 2018
Backport of #33932
suketa added a commit to suketa/rails_sandbox that referenced this pull request Jun 8, 2019
[close #33907] Error when using "recyclable" cache keys with a store
that does not support it

rails/rails#33932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.