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

Support deferred expiration of records and attributes for one-to-many associations #577

Merged
merged 7 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,27 @@ namespace :profile do
ruby "./performance/profile.rb"
end
end

namespace :db do
desc "Create the identity_cache_test database"
task :create do
require "mysql2"

config = {
host: ENV.fetch("MYSQL_HOST") || "localhost",
port: ENV.fetch("MYSQL_PORT") || 1037,
username: ENV.fetch("MYSQL_USER") || "root",
password: ENV.fetch("MYSQL_PASSWORD") || "",
}

begin
client = Mysql2::Client.new(config)
client.query("CREATE DATABASE IF NOT EXISTS identity_cache_test")
puts "Database 'identity_cache_test' created successfully. host: #{config[:host]}, port: #{config[:port]}"
rescue Mysql2::Error => e
puts "Error creating database: #{e.message}"
ensure
client&.close
end
end
end
16 changes: 10 additions & 6 deletions dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ up:
- bundler
- memcached
- mysql
- custom:
name: create database identity_cache_test
met?: mysql -u root -h 127.0.0.1 -P 1037 -e "SHOW DATABASES;" | grep identity_cache_test
meet: bundle exec rake db:create

commands:
test:
syntax:
optional:
argument: file
optional: args...
desc: 'Run tests'
desc: "Run tests"
run: |
if [[ $# -eq 0 ]]; then
bundle exec rake test
Expand All @@ -21,21 +25,21 @@ commands:
fi

style:
desc: 'Run rubocop checks'
desc: "Run rubocop checks"
run: bundle exec rubocop "$@"

check:
desc: 'Run tests and style checks'
desc: "Run tests and style checks"
run: bundle exec rake test && bundle exec rubocop

benchmark-cpu:
desc: 'Run the identity cache CPU benchmark'
desc: "Run the identity cache CPU benchmark"
run: bundle exec rake benchmark:cpu

profile:
desc: 'Profile IDC code'
desc: "Profile IDC code"
run: bundle exec rake profile:run

update-serialization-format:
desc: 'Update serialization format test fixture'
desc: "Update serialization format test fixture"
run: bundle exec rake update_serialization_format
45 changes: 27 additions & 18 deletions lib/identity_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class AssociationError < StandardError; end

class InverseAssociationError < StandardError; end

class NestedDeferredParentBlockError < StandardError; end
class NestedDeferredCacheExpirationBlockError < StandardError; end

class UnsupportedScopeError < StandardError; end

Expand Down Expand Up @@ -202,42 +202,51 @@ def fetch_multi(*keys)
result
end

# Executes a block with deferred parent expiration, ensuring that the parent
# records' cache expiration is deferred until the block completes. When the block
# completes, it triggers expiration of the primary index for the parent records.
# Raises a NestedDeferredParentBlockError if a deferred parent expiration block
# is already active on the current thread.
# Executes a block with deferred cache expiration, ensuring that the records' (parent,
# children and attributes) cache expiration is deferred until the block completes. When
# the block completes, it issues delete_multi calls for all the records and attributes
# that were marked for expiration.
#
# == Parameters:
# No parameters.
#
# == Raises:
# NestedDeferredParentBlockError if a deferred parent expiration block is already active.
# NestedDeferredCacheExpirationBlockError if a deferred cache expiration block is already active.
#
# == Yield:
# Runs the provided block with deferred parent expiration.
# Runs the provided block with deferred cache expiration.
#
# == Returns:
# The result of executing the provided block.
#
# == Ensures:
# Cleans up thread-local variables related to deferred parent expiration regardless
# Cleans up thread-local variables related to deferred cache expiration regardless
# of whether the block raises an exception.
def with_deferred_parent_expiration
raise NestedDeferredParentBlockError if Thread.current[:idc_deferred_parent_expiration]
def with_deferred_expiration
raise NestedDeferredCacheExpirationBlockError if Thread.current[:idc_deferred_expiration]

Thread.current[:idc_deferred_parent_expiration] = true
Thread.current[:idc_parent_records_for_cache_expiry] = Set.new
Thread.current[:idc_deferred_expiration] = true
Thread.current[:idc_records_to_expire] = Set.new
Thread.current[:idc_attributes_to_expire] = Set.new

result = yield

Thread.current[:idc_deferred_parent_expiration] = nil
Thread.current[:idc_parent_records_for_cache_expiry].each(&:expire_primary_index)

Thread.current[:idc_deferred_expiration] = nil
if Thread.current[:idc_records_to_expire].any?
IdentityCache.cache.delete_multi(
Thread.current[:idc_records_to_expire]
)
end
if Thread.current[:idc_attributes_to_expire].any?
IdentityCache.cache.delete_multi(
Thread.current[:idc_attributes_to_expire]
)
end
result
ensure
Thread.current[:idc_deferred_parent_expiration] = nil
Thread.current[:idc_parent_records_for_cache_expiry].clear
Thread.current[:idc_deferred_expiration] = nil
Thread.current[:idc_records_to_expire].clear
Thread.current[:idc_attributes_to_expire].clear
end

def with_fetch_read_only_records(value = true)
Expand Down
9 changes: 9 additions & 0 deletions lib/identity_cache/cache_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ def delete(key)
@cache_backend.write(key, IdentityCache::DELETED, expires_in: IdentityCache::DELETED_TTL.seconds)
end

def delete_multi(keys)
drinkbeer marked this conversation as resolved.
Show resolved Hide resolved
key_values = keys.map { |key| [key, IdentityCache::DELETED] }.to_h
@cache_backend.write_multi(key_values, expires_in: IdentityCache::DELETED_TTL.seconds)
end

def clear
@cache_backend.clear
end
Expand All @@ -82,6 +87,10 @@ def fetch(key, fill_lock_duration: nil, lock_wait_tries: 2, &block)
end
end

def exist?(key)
@cache_backend.exist?(key)
end

private

def fetch_without_fill_lock(key)
Expand Down
18 changes: 15 additions & 3 deletions lib/identity_cache/cached/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,24 @@ def expire(record)

unless record.send(:was_new_record?)
old_key = old_cache_key(record)
all_deleted = IdentityCache.cache.delete(old_key)

if Thread.current[:idc_deferred_expiration]
Thread.current[:idc_attributes_to_expire] << old_key
# defer the deletion, and don't block the following deletion
all_deleted = true
else
all_deleted = IdentityCache.cache.delete(old_key)
end
end
unless record.destroyed?
new_key = new_cache_key(record)
if new_key != old_key
all_deleted = IdentityCache.cache.delete(new_key) && all_deleted
if Thread.current[:idc_deferred_expiration]
Thread.current[:idc_attributes_to_expire] << new_key
all_deleted = true
else
all_deleted = IdentityCache.cache.delete(new_key) && all_deleted
end
end
end

Expand Down Expand Up @@ -152,9 +164,9 @@ def new_cache_key(record)
end

def old_cache_key(record)
changes = record.transaction_changed_attributes
old_key_values = key_fields.map do |field|
field_string = field.to_s
changes = record.transaction_changed_attributes
if record.destroyed? && changes.key?(field_string)
changes[field_string]
elsif record.persisted? && changes.key?(field_string)
Expand Down
6 changes: 5 additions & 1 deletion lib/identity_cache/cached/primary_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ def fetch_multi(ids)

def expire(id)
id = cast_id(id)
IdentityCache.cache.delete(cache_key(id))
if Thread.current[:idc_deferred_expiration]
Thread.current[:idc_records_to_expire] << cache_key(id)
else
IdentityCache.cache.delete(cache_key(id))
end
end

def cache_key(id)
Expand Down
14 changes: 14 additions & 0 deletions lib/identity_cache/memoized_cache_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ def delete(key)
end
end

def delete_multi(keys)
memoizing = memoizing?
ActiveSupport::Notifications.instrument("cache_delete_multi.identity_cache", memoizing: memoizing) do
if memoizing
keys.each { |key| memoized_key_values.delete(key) }
end
@cache_fetcher.delete_multi(keys)
end
end

def fetch(key, cache_fetcher_options = {}, &block)
memo_misses = 0
cache_misses = 0
Expand Down Expand Up @@ -137,6 +147,10 @@ def clear
end
end

def exist?(key)
@cache_fetcher.exist?(key)
end

private

EMPTY_ARRAY = [].freeze
Expand Down
4 changes: 0 additions & 4 deletions lib/identity_cache/parent_model_expiration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ def expire_parent_caches
add_parents_to_cache_expiry_set(parents_to_expire)
parents_to_expire.select! { |parent| parent.class.primary_cache_index_enabled }
parents_to_expire.reduce(true) do |all_expired, parent|
if Thread.current[:idc_deferred_parent_expiration]
Thread.current[:idc_parent_records_for_cache_expiry] << parent
next parent
end
Comment on lines -50 to -53
Copy link
Contributor Author

@drinkbeer drinkbeer Oct 9, 2024

Choose a reason for hiding this comment

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

The parent.expire_primary_index underlying calls the primary_index.expire, so we have an if block in the primary_index.expire, we don't need separate Set here. Just make the parent and associated records in one Set, and call write_multi to the keys. It will make the code simpler and easier to understand.

parent.expire_primary_index && all_expired
end
end
Expand Down
46 changes: 23 additions & 23 deletions test/index_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,48 +166,47 @@ def test_unique_cache_index_with_non_id_primary_key
assert_equal(123, KeyedRecord.fetch_by_value("a").id)
end

def test_with_deferred_parent_expiration_expires_parent_index_once
def test_with_deferred_expiration_for_parent_records_expires_parent_index_once
Item.send(:cache_has_many, :associated_records, embed: true)

@parent = Item.create!(title: "bob")
@records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }])

@memcached_spy = Spy.on(backend, :write).and_call_through

@memcached_spy_write_multi = Spy.on(backend, :write_multi).and_call_through
expected_item_expiration_count = Array(@parent).count
expected_associated_record_expiration_count = @records.count

expected_return_value = "Some text that we expect to see returned from the block"

result = IdentityCache.with_deferred_parent_expiration do
result = IdentityCache.with_deferred_expiration do
@parent.transaction do
@parent.associated_records.destroy_all
end
assert_equal(expected_associated_record_expiration_count, @memcached_spy.calls.count)
expected_return_value
end

expired_cache_keys = @memcached_spy.calls.map(&:args).map(&:first)
item_expiration_count = expired_cache_keys.count { _1.include?("Item") }
associated_record_expiration_count = expired_cache_keys.count { _1.include?("AssociatedRecord") }
all_keys = @memcached_spy_write_multi.calls.flat_map { |call| call.args.first.keys }
item_expiration_count = all_keys.count { _1.include?(":blob:Item:") }
associated_record_expiration_count = all_keys.count { _1.include?(":blob:AssociatedRecord:") }

assert_operator(@memcached_spy.calls.count, :>, 0)
assert_equal(1, @memcached_spy_write_multi.calls.count)
assert_equal(expected_item_expiration_count, item_expiration_count)
assert_equal(expected_associated_record_expiration_count, associated_record_expiration_count)
assert_equal(expected_return_value, result)
end

def test_double_nested_deferred_parent_expiration_will_raise_error
def test_double_nested_deferred_expiration_for_parent_records_will_raise_error
Item.send(:cache_has_many, :associated_records, embed: true)

@parent = Item.create!(title: "bob")
@records = @parent.associated_records.create!([{ name: "foo" }, { name: "bar" }, { name: "baz" }])

@memcached_spy = Spy.on(backend, :write).and_call_through
@memcached_spy_write_multi = Spy.on(backend, :write_multi).and_call_through

assert_raises(IdentityCache::NestedDeferredParentBlockError) do
IdentityCache.with_deferred_parent_expiration do
IdentityCache.with_deferred_parent_expiration do
assert_raises(IdentityCache::NestedDeferredCacheExpirationBlockError) do
IdentityCache.with_deferred_expiration do
IdentityCache.with_deferred_expiration do
@parent.transaction do
@parent.associated_records.destroy_all
end
Expand All @@ -216,9 +215,10 @@ def test_double_nested_deferred_parent_expiration_will_raise_error
end

assert_equal(0, @memcached_spy.calls.count)
assert_equal(0, @memcached_spy_write_multi.calls.count)
end

def test_deep_association_with_deferred_parent_expiration_expires_parent_once
def test_deep_association_with_deferred_expiration_expires_parent_once
AssociatedRecord.send(:has_many, :deeply_associated_records, dependent: :destroy)
Item.send(:cache_has_many, :associated_records, embed: true)

Expand All @@ -232,27 +232,27 @@ def test_deep_association_with_deferred_parent_expiration_expires_parent_once
])
end

@memcached_spy = Spy.on(backend, :write).and_call_through
@memcached_spy_write_multi = Spy.on(backend, :write_multi).and_call_through

expected_item_expiration_count = Array(@parent).count
expected_associated_record_expiration_count = @records.count
expected_deeply_associated_record_expiration_count = @records.flat_map(&:deeply_associated_records).count

IdentityCache.with_deferred_parent_expiration do
IdentityCache.with_deferred_expiration do
@parent.transaction do
@parent.associated_records.destroy_all
end
end

expired_cache_keys = @memcached_spy.calls.map(&:args).map(&:first)
item_expiration_count = expired_cache_keys.count { _1.include?("Item") }
associated_record_expiration_count = expired_cache_keys.count { _1.include?(":AssociatedRecord:") }
deeply_associated_record_expiration_count = expired_cache_keys.count { _1.include?("DeeplyAssociatedRecord") }
all_keys = @memcached_spy_write_multi.calls.flat_map { |call| call.args.first.keys }
item_expiration_count = all_keys.count { |key| key.include?(":blob:Item:") }
associated_record_keys = all_keys.count { |key| key.include?(":blob:AssociatedRecord:") }
deeply_associated_record_keys = all_keys.count { |key| key.include?(":blob:DeeplyAssociatedRecord:") }

assert_operator(@memcached_spy.calls.count, :>, 0)
assert_equal(1, @memcached_spy_write_multi.calls.count)
assert_equal(expected_item_expiration_count, item_expiration_count)
assert_equal(expected_associated_record_expiration_count, associated_record_expiration_count)
assert_equal(expected_deeply_associated_record_expiration_count, deeply_associated_record_expiration_count)
assert_equal(expected_associated_record_expiration_count, associated_record_keys)
assert_equal(expected_deeply_associated_record_expiration_count, deeply_associated_record_keys)
end

private
Expand Down
Loading
Loading