From 0664c29202913ac0d90316c456a03775e5e340e4 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Mon, 11 Mar 2024 17:39:54 -0700 Subject: [PATCH] secondary timeout when hard coded timeout is more than 100x the expected p99, dynamically adjust that value to be more restrictive --- lib/network_resiliency.rb | 15 +++++++++------ spec/network_resiliency_spec.rb | 14 ++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/network_resiliency.rb b/lib/network_resiliency.rb index 36e5262..a2e0e2f 100644 --- a/lib/network_resiliency.rb +++ b/lib/network_resiliency.rb @@ -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( @@ -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", diff --git a/spec/network_resiliency_spec.rb b/spec/network_resiliency_spec.rb index 9fed189..1418453 100644 --- a/spec/network_resiliency_spec.rb +++ b/spec/network_resiliency_spec.rb @@ -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 } @@ -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 @@ -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 ] @@ -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