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

Skip the :race_condition_ttl branch if the option is 0 or nil. #19296

Merged
merged 1 commit into from
Mar 17, 2015

Conversation

Wildebeest
Copy link

This fixes an issue with the redis cache, where this code will sometimes throw an error out of SETEX when passing 0 as the expires_at.

… fixes an issue with the redis cache, where this code will sometimes throw an error out of SETEX when passing 0 as the `expires_at`.
@@ -566,8 +566,8 @@ def find_cached_entry(key, name, options)
def handle_expired_entry(entry, key, options)
if entry && entry.expired?
race_ttl = options[:race_condition_ttl].to_i
if race_ttl && (Time.now.to_f - entry.expires_at <= race_ttl)
# When an entry has :race_condition_ttl defined, put the stale entry back into the cache
if (race_ttl > 0) && (Time.now.to_f - entry.expires_at <= race_ttl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if race_ttl is nil it will now raise a NoMethodError

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

race_ttl = options[:race_condition_ttl].to_i will never be nil

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! nil.to_i evaluates to 0 and 0 is truthy in ruby.

ttl = nil.to_i
puts ttl
if ttl
  puts "I am a tautology."
end

Outputs

0
I am a tautology.

@Wildebeest
Copy link
Author

@rafaelfranca @egilburg Ping. Is there anything I need to do here?

@egilburg
Copy link
Contributor

@Wildebeest no it's ok.

I know 0 is truthy, I just didn't realize the old code already checked for it (meaning the old code before your patch didn't actually check anything meaningful, because essentially it always checked if (true) :D

@Wildebeest
Copy link
Author

Sweet. Thanks! :)

rafaelfranca added a commit that referenced this pull request Mar 17, 2015
Skip the `:race_condition_ttl` branch if the option is 0 or nil.
@rafaelfranca rafaelfranca merged commit a42c952 into rails:master Mar 17, 2015
rafaelfranca added a commit that referenced this pull request Mar 17, 2015
Skip the `:race_condition_ttl` branch if the option is 0 or nil.
rafaelfranca added a commit that referenced this pull request Mar 17, 2015
Skip the `:race_condition_ttl` branch if the option is 0 or nil.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants