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

Don't cache locally if unless_exist was passed #29083

Merged
merged 1 commit into from May 15, 2017

Conversation

@eugeneius
Copy link
Member

@eugeneius eugeneius commented May 14, 2017

Some cache backends support the unless_exist option, which tells them not to overwrite existing entries. The local cache currently always stores the new value, even though the backend may have rejected it.

Since we can't tell which value will end up in the backend cache, we should delete the key from the local cache, so that the next read for that key will go to the backend and pick up the correct value.

The DalliStore backend hacked around this problem in petergoldstein/dalli#365.

Some cache backends support the `unless_exist` option, which tells them
not to overwrite an existing entry. The local cache currently always
stores the new value, even though the backend may have rejected it.

Since we can't tell which value will end up in the backend cache, we
should delete the key from the local cache, so that the next read for
that key will go to the backend and pick up the correct value.
@rails-bot
Copy link

@rails-bot rails-bot commented May 14, 2017

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@eileencodes eileencodes merged commit 5bfb287 into rails:master May 15, 2017
2 checks passed
2 checks passed
codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eileencodes
Copy link
Member

@eileencodes eileencodes commented May 15, 2017

Thanks! 😄

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

4 participants