Increase request retry timebox from 4s to 8s and when Fallback Mode is disabled then retry request 6 times

CHANGELOG.md

@@ -1,5 +1,16 @@
 # Change Log
 
+### 1.22.0
+
+* Increase request retry timebox from 4s to 8s to not flood Knapsack Pro API with too many requests in a short period of time and to give time for API server to autoscale and add additional machines to serve traffic
+* When Fallback Mode is disabled with env `KNAPSACK_PRO_FALLBACK_MODE_ENABLED=false` then retry the request to Knapsack Pro API for 6 times instead of only 3 times.
+
+  Here is related [info why some users want to disable Fallback Mode](https://github.com/KnapsackPro/knapsack_pro-ruby#required-ci-configuration-if-you-use-retry-single-failed-ci-node-feature-on-your-ci-server-when-knapsack_pro_fixed_queue_splittrue-in-queue-mode-or-knapsack_pro_fixed_test_suite_splittrue-in-regular-mode).
+
+    https://github.com/KnapsackPro/knapsack_pro-ruby/pull/112
+
+https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v1.21.0...v1.22.0
+
 ### 1.21.0
 
 * Automatically detect slow test files for RSpec and split them by test examples when `KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true`

lib/knapsack_pro/client/connection.rb

@@ -4,8 +4,8 @@ class Connection
       class ServerError < StandardError; end
 
       TIMEOUT = 15
-      MAX_RETRY = 3
-      REQUEST_RETRY_TIMEBOX = 4
+      MAX_RETRY = -> { KnapsackPro::Config::Env.fallback_mode_enabled? ? 3 : 6 }
+      REQUEST_RETRY_TIMEBOX = 8
 
       def initialize(action)
         @action = action
@@ -118,7 +118,7 @@ def make_request(&block)
       rescue ServerError, Errno::ECONNREFUSED, Errno::ETIMEDOUT, Errno::EPIPE, EOFError, SocketError, Net::OpenTimeout, Net::ReadTimeout, OpenSSL::SSL::SSLError => e
         logger.warn(e.inspect)
         retries += 1
-        if retries < MAX_RETRY
+        if retries < MAX_RETRY.call
           wait = retries * REQUEST_RETRY_TIMEBOX
           logger.warn("Wait #{wait}s and retry request to Knapsack Pro API.")
           Kernel.sleep(wait)

spec/knapsack_pro/client/connection_spec.rb

@@ -72,29 +72,65 @@
 
     before do
       expect(KnapsackPro).to receive(:logger).at_least(1).and_return(logger)
+    end
+
+    it do
       expect(logger).to receive(:debug).exactly(3).with("#{expected_http_method} http://api.knapsackpro.test:3000/v1/fake_endpoint")
       expect(logger).to receive(:debug).exactly(3).with('API request UUID: fake-uuid')
       expect(logger).to receive(:debug).exactly(3).with('API response:')
-    end
 
-    it do
       parsed_response = { 'error' => 'Internal Server Error' }
 
       expect(logger).to receive(:error).exactly(3).with(parsed_response)
 
       server_error = described_class::ServerError.new(parsed_response)
       expect(logger).to receive(:warn).exactly(3).with(server_error.inspect)
 
-      expect(logger).to receive(:warn).with("Wait 4s and retry request to Knapsack Pro API.")
       expect(logger).to receive(:warn).with("Wait 8s and retry request to Knapsack Pro API.")
-      expect(Kernel).to receive(:sleep).with(4)
+      expect(logger).to receive(:warn).with("Wait 16s and retry request to Knapsack Pro API.")
       expect(Kernel).to receive(:sleep).with(8)
+      expect(Kernel).to receive(:sleep).with(16)
 
       expect(subject).to eq(parsed_response)
 
       expect(connection.success?).to be false
       expect(connection.errors?).to be true
     end
+
+    context 'when Fallback Mode is disabled' do
+      before do
+        expect(KnapsackPro::Config::Env).to receive(:fallback_mode_enabled?).at_least(1).and_return(false)
+      end
+
+      it do
+        expect(logger).to receive(:debug).exactly(6).with("#{expected_http_method} http://api.knapsackpro.test:3000/v1/fake_endpoint")
+        expect(logger).to receive(:debug).exactly(6).with('API request UUID: fake-uuid')
+        expect(logger).to receive(:debug).exactly(6).with('API response:')
+
+        parsed_response = { 'error' => 'Internal Server Error' }
+
+        expect(logger).to receive(:error).exactly(6).with(parsed_response)
+
+        server_error = described_class::ServerError.new(parsed_response)
+        expect(logger).to receive(:warn).exactly(6).with(server_error.inspect)
+
+        expect(logger).to receive(:warn).with("Wait 8s and retry request to Knapsack Pro API.")
+        expect(logger).to receive(:warn).with("Wait 16s and retry request to Knapsack Pro API.")
+        expect(logger).to receive(:warn).with("Wait 24s and retry request to Knapsack Pro API.")
+        expect(logger).to receive(:warn).with("Wait 32s and retry request to Knapsack Pro API.")
+        expect(logger).to receive(:warn).with("Wait 40s and retry request to Knapsack Pro API.")
+        expect(Kernel).to receive(:sleep).with(8)
+        expect(Kernel).to receive(:sleep).with(16)
+        expect(Kernel).to receive(:sleep).with(24)
+        expect(Kernel).to receive(:sleep).with(32)
+        expect(Kernel).to receive(:sleep).with(40)
+
+        expect(subject).to eq(parsed_response)
+
+        expect(connection.success?).to be false
+        expect(connection.errors?).to be true
+      end
+    end
   end
 end
 
@@ -186,7 +222,7 @@
       let(:http_method) { :post }
 
       before do
-        expect(http).to receive(:post).exactly(3).with(
+        expect(http).to receive(:post).at_least(3).with(
           endpoint_path,
           request_hash.to_json,
           {
@@ -210,7 +246,7 @@
       before do
         uri = URI.parse("http://api.knapsackpro.test:3000#{endpoint_path}")
         uri.query = URI.encode_www_form(request_hash)
-        expect(http).to receive(:get).exactly(3).with(
+        expect(http).to receive(:get).at_least(3).with(
           uri,
           {
             'Content-Type' => 'application/json',