Skip to content

Commit

Permalink
secondary timeout
Browse files Browse the repository at this point in the history
when hard coded timeout is more than 100x the expected p99, dynamically adjust that value to be more restrictive
  • Loading branch information
dpep committed Mar 12, 2024
1 parent bbd3984 commit 0664c29
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 14 deletions.
15 changes: 9 additions & 6 deletions lib/network_resiliency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,16 @@ def timeouts_for(adapter:, action:, destination:, max: nil, units: :ms)
if p99 < max
timeouts << p99

# fallback attempt
if max - p99 > p99
# make a second, more lenient attempt

if p99 * 100 < max
# max is excessively high
timeouts << p99 * 100
elsif p99 * 10 < max
# use remaining time for second attempt
timeouts << max - p99
else
# max is smallish
timeouts << max

NetworkResiliency.statsd&.increment(
Expand All @@ -385,10 +390,8 @@ def timeouts_for(adapter:, action:, destination:, max: nil, units: :ms)
else
timeouts << p99

# timeouts << p99 * 10 if NetworkResiliency.mode(action) == :resolute

# unbounded second attempt
timeouts << nil
# second attempt
timeouts << p99 * 100

NetworkResiliency.statsd&.increment(
"network_resiliency.timeout.missing",
Expand Down
14 changes: 6 additions & 8 deletions spec/network_resiliency_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ def record
end
let(:n) { described_class::RESILIENCY_THRESHOLD }
let(:p99) { 10 }
let(:max) { 100 }
let(:max) { 1_000 }
let(:units) { nil }
let(:timeout_min) { 0 }

Expand Down Expand Up @@ -719,15 +719,15 @@ def record
let(:p99) { 9 }

it "generates more granular timeouts" do
is_expected.to eq [ p99, max - p99 ]
is_expected.to eq [ p99, p99 * 100 ]
end
end

context "when there is no max timeout" do
let(:max) { nil }

it "should make one attempt with a timeout and one unbounded attempt" do
is_expected.to eq [ p99, nil ]
it "should make two attempts with timeouts" do
is_expected.to eq [ p99, p99 * 100 ]
end
end

Expand Down Expand Up @@ -755,9 +755,7 @@ def record
end

context "when the max timeout is similarly sized to the p99" do
let(:max) { p99 * 1.5 }

specify { expect(max - p99).to be < p99 }
let(:max) { p99 * 10 }

it "makes two attempts, using the max as the second" do
is_expected.to eq [ p99, max ]
Expand All @@ -777,7 +775,7 @@ def record
let(:timeout_min) { 50 }

it "falls back to the timeout_min" do
is_expected.to eq [ described_class.timeout_min, max ]
is_expected.to eq [ described_class.timeout_min, max - timeout_min ]
end
end

Expand Down

0 comments on commit 0664c29

Please sign in to comment.