Merge branch 'stomlinson/force-service-discovery-on-startup' into 'master'

Ensure service discovery runs before secondaries are used

See merge request gitlab-org/gitlab!67899

lib/gitlab/database/load_balancing/service_discovery.rb

@@ -63,31 +63,36 @@ module Gitlab
         end
 
         def start
+          # We run service discovery once in the current thread so that the application's main thread
+          # does not race this thread to use the results of initial service discovery.
+          next_sleep_duration = perform_service_discovery
+
           Thread.new do
             loop do
-              interval =
-                begin
-                  refresh_if_necessary
-                rescue StandardError => error
-                  # Any exceptions that might occur should be reported to
-                  # Sentry, instead of silently terminating this thread.
-                  Gitlab::ErrorTracking.track_exception(error)
-
-                  Gitlab::AppLogger.error(
-                    "Service discovery encountered an error: #{error.message}"
-                  )
-
-                  self.interval
-                end
-
               # We slightly randomize the sleep() interval. This should reduce
               # the likelihood of _all_ processes refreshing at the same time,
               # possibly putting unnecessary pressure on the DNS server.
-              sleep(interval + rand(MAX_SLEEP_ADJUSTMENT))
+              sleep(next_sleep_duration + rand(MAX_SLEEP_ADJUSTMENT))
+
+              next_sleep_duration = perform_service_discovery
             end
           end
         end
 
+        def perform_service_discovery
+          refresh_if_necessary
+        rescue StandardError => error
+          # Any exceptions that might occur should be reported to
+          # Sentry, instead of silently terminating this thread.
+          Gitlab::ErrorTracking.track_exception(error)
+
+          Gitlab::AppLogger.error(
+            "Service discovery encountered an error: #{error.message}"
+          )
+
+          interval
+        end
+
         # Refreshes the hosts, but only if the DNS record returned a new list of
         # addresses.
         #

spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb

@@ -57,22 +57,21 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do
         .and_yield
     end
 
-    it 'starts service discovery in a new thread' do
-      expect(service)
-        .to receive(:refresh_if_necessary)
-        .and_return(5)
+    it 'runs service discovery once before starting the worker thread' do
+      expect(service).to receive(:perform_service_discovery).ordered.and_return(5)
 
-      expect(service)
-        .to receive(:rand)
-        .and_return(2)
+      expect(Thread).to receive(:new).ordered.and_call_original # Thread starts
 
-      expect(service)
-        .to receive(:sleep)
-        .with(7)
+      expect(service).to receive(:rand).ordered.and_return(2)
+      expect(service).to receive(:sleep).ordered.with(7) # Sleep runs after thread starts
+
+      expect(service).to receive(:perform_service_discovery).ordered.and_return(1)
 
       service.start.join
     end
+  end
 
+  describe '#perform_service_discovery' do
     it 'reports exceptions to Sentry' do
       error = StandardError.new
 
@@ -84,15 +83,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do
         .to receive(:track_exception)
         .with(error)
 
-      expect(service)
-        .to receive(:rand)
-        .and_return(2)
-
-      expect(service)
-        .to receive(:sleep)
-        .with(62)
-
-      service.start.join
+      service.perform_service_discovery
     end
   end