Ensure service discovery runs before results are used

Fixes a race condition where database queries could start before the
first round of service discovery completes. By running service discovery
once on the main thread before starting the worker thread, we can be
sure that it runs at least once before the application needs the
results.

Changelog: fixed

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