Merge branch '221239-geo-file-downloads-can-block-sidekiq-threads' into 'master'

Fix: Geo file downloads can block Sidekiq threads

Closes #221239

See merge request gitlab-org/gitlab!36012

ee/changelogs/unreleased/221239-geo-file-downloads-can-block-sidekiq-threads.yml

+---
+title: 'Fix: Geo file downloads can block Sidekiq threads'
+merge_request:
+author:
+type: fixed

ee/lib/gitlab/geo/replication/base_transfer.rb

@@ -8,7 +8,12 @@ module Gitlab
 
         attr_reader :file_type, :file_id, :filename, :expected_checksum, :request_data, :resource
 
-        TEMP_PREFIX = 'tmp_'.freeze
+        TEMP_PREFIX = 'tmp_'
+        DOWNLOAD_TIMEOUT = {
+          connect: 60,
+          write: 60,
+          read: 60
+        }.freeze
 
         def initialize(resource:, file_type:, file_id:, request_data:, expected_checksum: nil, filename: nil)
           @resource = resource
@@ -128,7 +133,7 @@ module Gitlab
 
           begin
             # Make the request
-            response = ::HTTP.get(url, headers: req_headers)
+            response = ::HTTP.timeout(DOWNLOAD_TIMEOUT.dup).get(url, headers: req_headers)
 
             # Check for failures
             unless response.status.success?
@@ -183,7 +188,7 @@ module Gitlab
 
           begin
             # Make the request
-            response = ::HTTP.follow.get(url, headers: req_headers)
+            response = ::HTTP.timeout(DOWNLOAD_TIMEOUT.dup).follow.get(url, headers: req_headers)
 
             # Check for failures
             unless response.status.success?

ee/spec/lib/gitlab/geo/replication/base_transfer_spec.rb

@@ -23,6 +23,29 @@ RSpec.describe Gitlab::Geo::Replication::BaseTransfer do
     end
   end
 
+  describe 'timeout' do
+    subject do
+      described_class.new(file_type: :avatar, file_id: 1, filename: Tempfile.new,
+                          expected_checksum: nil, request_data: nil, resource: nil)
+    end
+
+    before do
+      stub_current_geo_node(secondary_node)
+    end
+
+    it 'sets a timeout when downbloads to local storage' do
+      expect(::HTTP).to receive(:timeout)
+
+      subject.download_from_primary
+    end
+
+    it 'sets a timeout when streaming to object storage' do
+      expect(::HTTP).to receive(:timeout)
+
+      subject.stream_from_primary_to_object_storage
+    end
+  end
+
   describe '#can_transfer?' do
     subject do
       described_class.new(file_type: :avatar, file_id: 1, filename: Tempfile.new,