Skip to content

Commit 6014aa5

Browse files
committed
Catch errors from the Dalli client
Sometimes under heavy load the Dalli client is raising exceptions so catch those to return either nil or false.
1 parent 2ac64b3 commit 6014aa5

2 files changed

Lines changed: 80 additions & 18 deletions

File tree

app/lib/active_support/cache/atomic_dalli_store.rb

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,11 @@ def read(name, options = nil)
3434
def write(name, value, options = nil)
3535
expiry = (options && options[:expires_in]) || 0
3636
options[:expires_in] = expiry + 20 unless expiry.zero?
37-
ttl_set(ttl_key(name, options), expiry)
38-
super
37+
ttl_set(ttl_key(name, options), expiry) && super
3938
end
4039

4140
def delete(name, options = nil)
42-
ttl_delete(ttl_key(name, options))
43-
super
41+
ttl_delete(ttl_key(name, options)) && super
4442
end
4543

4644
private
@@ -56,18 +54,34 @@ def ttl_key(name, options)
5654

5755
def ttl_get(key)
5856
with { |c| c.get(key, raw: true) }
57+
rescue Dalli::DalliError => e
58+
logger.error("DalliError: #{e.message}") if logger
59+
raise if raise_errors?
60+
nil
5961
end
6062

6163
def ttl_add(key)
6264
with { |c| c.add(key, "", 10, raw: true) }
65+
rescue Dalli::DalliError => e
66+
logger.error("DalliError: #{e.message}") if logger
67+
raise if raise_errors?
68+
false
6369
end
6470

6571
def ttl_set(key, expiry)
6672
with { |c| c.set(key, "", expiry, raw: true) }
73+
rescue Dalli::DalliError => e
74+
logger.error("DalliError: #{e.message}") if logger
75+
raise if raise_errors?
76+
false
6777
end
6878

6979
def ttl_delete(key)
7080
with { |c| c.delete(key) }
81+
rescue Dalli::DalliError => e
82+
logger.error("DalliError: #{e.message}") if logger
83+
raise if raise_errors?
84+
false
7185
end
7286
end
7387
end

spec/lib/atomic_dalli_store_spec.rb

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,28 @@
22

33
RSpec.describe ActiveSupport::Cache::AtomicDalliStore do
44
let(:options) do
5-
{ namespace: "epets_test", expires_in: 5.minutes }
5+
{ namespace: "epets_test", expires_in: 2.seconds }
66
end
77

8+
let(:client) { subject.dalli }
9+
let(:exception) { Dalli::DalliError.new }
10+
11+
let(:ttl_key) { "epets_test:foo.ttl" }
12+
let(:ttl_set_args) { [ttl_key, "", 2.seconds, raw: true] }
13+
let(:ttl_get_args) { [ttl_key, raw: true] }
14+
let(:ttl_add_args) { [ttl_key, "", 10, raw: true] }
15+
816
around do |example|
9-
subject.with do |client|
10-
@client = client
11-
@client.delete("epets_test:foo")
12-
@client.delete("epets_test:foo.ttl")
17+
client.delete("epets_test:foo")
18+
client.delete("epets_test:foo.ttl")
19+
example.run
20+
end
1321

14-
example.run
15-
end
22+
before do
23+
allow(client).to receive(:get).and_call_original
24+
allow(client).to receive(:set).and_call_original
25+
allow(client).to receive(:add).and_call_original
26+
allow(client).to receive(:delete).and_call_original
1627
end
1728

1829
describe "#fetch" do
@@ -27,21 +38,26 @@
2738
expect {
2839
subject.fetch("foo", options) { "bar" }
2940
}.to change {
30-
@client.get("epets_test:foo")
41+
client.get("epets_test:foo")
3142
}.from(nil).to("bar")
3243
end
3344

3445
it "writes the TTL value to the cache" do
3546
expect {
3647
subject.fetch("foo", options) { "bar" }
3748
}.to change {
38-
@client.get("epets_test:foo.ttl")
49+
client.get("epets_test:foo.ttl")
3950
}.from(nil).to("")
4051
end
4152

4253
it "returns the value" do
4354
expect(subject.fetch("foo", options) { "bar" }).to eq("bar")
4455
end
56+
57+
it "handles exceptions" do
58+
expect(subject.dalli).to receive(:set).with(*ttl_set_args).and_raise(exception)
59+
expect(subject.fetch("foo", options) { "bar" }).to eq("bar")
60+
end
4561
end
4662

4763
context "when the cache is set" do
@@ -58,13 +74,24 @@
5874
it "returns the value" do
5975
expect(subject.fetch("foo", options) { "bar" }).to eq("bar")
6076
end
77+
78+
it "handles exceptions when reading the lock" do
79+
expect(subject.dalli).to receive(:get).with(*ttl_get_args).and_raise(exception)
80+
expect(subject.fetch("foo", options) { "bar" }).to eq("bar")
81+
end
82+
83+
it "handles exceptions when setting the lock" do
84+
client.delete(ttl_key)
85+
expect(subject.dalli).to receive(:add).with(*ttl_add_args).and_raise(exception)
86+
expect(subject.fetch("foo", options) { "bar" }).to eq("bar")
87+
end
6188
end
6289
end
6390

6491
describe "#read" do
6592
context "when the cache is not set" do
6693
it "returns nil" do
67-
expect(subject.read("foo")).to be_nil
94+
expect(subject.read("foo", options)).to be_nil
6895
end
6996
end
7097

@@ -76,6 +103,17 @@
76103
it "returns the value" do
77104
expect(subject.read("foo", options)).to eq("bar")
78105
end
106+
107+
it "handles exceptions when reading the lock" do
108+
expect(subject.dalli).to receive(:get).with(*ttl_get_args).and_raise(exception)
109+
expect(subject.read("foo", options)).to eq("bar")
110+
end
111+
112+
it "handles exceptions when setting the lock" do
113+
client.delete(ttl_key)
114+
expect(subject.dalli).to receive(:add).with(*ttl_add_args).and_raise(exception)
115+
expect(subject.read("foo", options)).to eq("bar")
116+
end
79117
end
80118
end
81119

@@ -84,17 +122,22 @@
84122
expect {
85123
subject.write("foo", "bar", options)
86124
}.to change {
87-
@client.get("epets_test:foo")
125+
client.get("epets_test:foo")
88126
}.from(nil).to("bar")
89127
end
90128

91129
it "writes the TTL value to the cache" do
92130
expect {
93131
subject.write("foo", "bar", options)
94132
}.to change {
95-
@client.get("epets_test:foo.ttl")
133+
client.get("epets_test:foo.ttl")
96134
}.from(nil).to("")
97135
end
136+
137+
it "handles exceptions when setting the TTL" do
138+
expect(subject.dalli).to receive(:set).with(*ttl_set_args).and_raise(exception)
139+
expect(subject.write("foo", "bar", options)).to be_falsey
140+
end
98141
end
99142

100143
describe "#delete" do
@@ -106,16 +149,21 @@
106149
expect {
107150
subject.delete("foo", options)
108151
}.to change {
109-
@client.get("epets_test:foo")
152+
client.get("epets_test:foo")
110153
}.from("bar").to(nil)
111154
end
112155

113156
it "deletes the TTL value from the cache" do
114157
expect {
115158
subject.delete("foo", options)
116159
}.to change {
117-
@client.get("epets_test:foo.ttl")
160+
client.get("epets_test:foo.ttl")
118161
}.from("").to(nil)
119162
end
163+
164+
it "handles exceptions when deleting the TTL" do
165+
expect(subject.dalli).to receive(:delete).with(ttl_key).and_raise(exception)
166+
expect(subject.delete("foo", options)).to be_falsey
167+
end
120168
end
121169
end

0 commit comments

Comments
 (0)