Skip to content

Commit f99e867

Browse files
committed
assume arg to Account.invalidate_cache is relative to current shard
instead of assuming it's relative to the birth shard Note: this means that the call to invalidate the cache before an account is saved should start working, which may increase the number of cache invalidations on this key. fixes CNVS-25290 test plan: - in a rails console, activate a shard that is not the birth shard - Call `Account.find_cached(<local_id>)`, notice the db query - Call `Account.invalidate_cache(<local_id>)`, should return true or 1 indicating that it successfully deleted something from the cache Change-Id: I1da7a6ab4eb457d9bc4287f2294aebcb2f9c825e Reviewed-on: https://gerrit.instructure.com/67732 Reviewed-by: Cody Cutrer <cody@instructure.com> Tested-by: Jenkins Reviewed-by: Jacob Fugal <jacob@instructure.com> QA-Review: August Thornton <august@instructure.com> Product-Review: Simon Williams <simon@instructure.com>
1 parent 2123950 commit f99e867

2 files changed

Lines changed: 29 additions & 12 deletions

File tree

app/models/account.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,10 @@ def self.account_lookup_cache_key(id)
525525
end
526526

527527
def self.invalidate_cache(id)
528+
return unless id
529+
birth_id = Shard.relative_id_for(id, Shard.current, Shard.birth)
528530
Shard.birth.activate do
529-
Rails.cache.delete(account_lookup_cache_key(id)) if id
531+
Rails.cache.delete(account_lookup_cache_key(birth_id)) if birth_id
530532
end
531533
rescue
532534
nil

spec/models/account_spec.rb

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,6 @@
2020

2121
describe Account do
2222

23-
describe ".find_cached" do
24-
specs_require_sharding
25-
26-
it "works relative to a different shard" do
27-
@shard1.activate do
28-
a = Account.create!
29-
expect(Account.find_cached(a.id)).to eq a
30-
end
31-
end
32-
end
33-
3423
it "should provide a list of courses" do
3524
expect{ Account.new.courses }.not_to raise_error
3625
end
@@ -1407,4 +1396,30 @@ def account_with_admin_and_restricted_user(account, restricted_role)
14071396
end
14081397
end
14091398
end
1399+
1400+
context "account cache" do
1401+
specs_require_sharding
1402+
1403+
describe ".find_cached" do
1404+
it "works relative to a different shard" do
1405+
@shard1.activate do
1406+
a = Account.create!
1407+
expect(Account.find_cached(a.id)).to eq a
1408+
end
1409+
end
1410+
end
1411+
1412+
describe ".invalidate_cache" do
1413+
it "works relative to a different shard" do
1414+
enable_cache do
1415+
@shard1.activate do
1416+
a = Account.create!
1417+
Account.find_cached(a.id) # set the cache
1418+
expect(Account.invalidate_cache(a.id)).to eq true
1419+
end
1420+
end
1421+
end
1422+
end
1423+
end
1424+
14101425
end

0 commit comments

Comments
 (0)