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

Jump to bottom

ActiveSupport#Cache :race_condition_ttl option handling not optimal #14157

Closed
wants to merge 3 commits into from
Closed

ActiveSupport#Cache :race_condition_ttl option handling not optimal #14157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b640bfe
ActiveSupport#Cache :race_condition_ttl option handling somewhat inef…
niclas Feb 22, 2014
3b089e9
Attempt to better handling of :race_condition_ttl.
niclas Feb 22, 2014
844b5e5
Add disarmed race_condition_protection test.
niclas Feb 22, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion activesupport/lib/active_support/cache.rb
View file
Open in desktop
Expand Up @@ -564,7 +564,7 @@ def find_cached_entry(key, name, options)
def handle_expired_entry(entry, key, options) def handle_expired_entry(entry, key, options)
if entry && entry.expired? if entry && entry.expired?
race_ttl = options[:race_condition_ttl].to_i race_ttl = options[:race_condition_ttl].to_i
if race_ttl && (Time.now.to_f - entry.expires_at <= race_ttl) if race_ttl > 0 && entry.time_since_expired <= race_ttl
# When an entry has :race_condition_ttl defined, put the stale entry back into the cache # When an entry has :race_condition_ttl defined, put the stale entry back into the cache
# for a brief period while the entry is begin recalculated. # for a brief period while the entry is begin recalculated.
entry.expires_at = Time.now + race_ttl entry.expires_at = Time.now + race_ttl
Expand Down Expand Up @@ -639,6 +639,10 @@ def expires_at=(value)
@expires_in = nil @expires_in = nil
end end
end end

def time_since_expired
Time.now.to_f - expires_at if expired?
end


# Returns the size of the cached value. This could be less than # Returns the size of the cached value. This could be less than
# <tt>value.size</tt> if the data is compressed. # <tt>value.size</tt> if the data is compressed.
Expand Down
26 changes: 24 additions & 2 deletions activesupport/test/caching_test.rb
View file
Open in desktop
Expand Up @@ -409,7 +409,7 @@ def test_race_condition_protection
end end
assert_equal "baz", result assert_equal "baz", result
end end

def test_race_condition_protection_is_limited def test_race_condition_protection_is_limited
time = Time.now time = Time.now
@cache.write('foo', 'bar', :expires_in => 60) @cache.write('foo', 'bar', :expires_in => 60)
Expand All @@ -420,7 +420,7 @@ def test_race_condition_protection_is_limited
end end
assert_equal "baz", result assert_equal "baz", result
end end

def test_race_condition_protection_is_safe def test_race_condition_protection_is_safe
time = Time.now time = Time.now
@cache.write('foo', 'bar', :expires_in => 60) @cache.write('foo', 'bar', :expires_in => 60)
Expand All @@ -437,6 +437,17 @@ def test_race_condition_protection_is_safe
assert_nil @cache.read('foo') assert_nil @cache.read('foo')
end end


def test_race_condition_protection_is_truly_disarmed_for_nil
time = Time.now
Time.stubs(:now).returns(time)
@cache.write('foo', 'bar', :expires_in => 60)
@cache.expects(:delete_entry)
Time.stubs(:now).returns(time + 60)
@cache.fetch('foo', :race_condition_ttl => nil) do
'baz'
end
end

def test_crazy_key_characters def test_crazy_key_characters
crazy_key = "#/:*(<+=> )&$%@?;'\"\'`~-" crazy_key = "#/:*(<+=> )&$%@?;'\"\'`~-"
assert @cache.write(crazy_key, "1", :raw => true) assert @cache.write(crazy_key, "1", :raw => true)
Expand Down Expand Up @@ -1029,6 +1040,17 @@ def test_expired
assert entry.expired?, 'entry is expired' assert entry.expired?, 'entry is expired'
end end


def test_time_since_expired
time = Time.now
Time.stubs(:now).returns(time)
entry = ActiveSupport::Cache::Entry.new("value", :expires_in => 60)
assert_equal nil, entry.time_since_expired
Time.stubs(:now).returns(time + 60)
assert_equal 0.0, entry.time_since_expired
Time.stubs(:now).returns(time + 61)
assert_equal 1.0, entry.time_since_expired
end

def test_compress_values def test_compress_values
value = "value" * 100 value = "value" * 100
entry = ActiveSupport::Cache::Entry.new(value, :compress => true, :compress_threshold => 1) entry = ActiveSupport::Cache::Entry.new(value, :compress => true, :compress_threshold => 1)
Expand Down