Merge pull request #3015 from zendesk/grosser/autoload

avoid autoloading errors by not running initial periodical iteration …

app/models/restart_signal_handler.rb

@@ -2,6 +2,7 @@
 # Ensures that we wait for all jobs to finish before shutting down the process during restart.
 # JobQueue locks a mutex, hence the need for a separate SignalHandler thread
 # Self-pipe is also best practice, since signal handlers can themselves be interrupted
+
 class RestartSignalHandler
   class << self
     alias_method :listen, :new

lib/error_notifier.rb

@@ -7,7 +7,7 @@ class << self
     def notify(exception, options = {})
       debug_info = Samson::Hooks.fire(:error, exception, options).compact.detect { |result| result.is_a?(String) }
       if ::Rails.env.test?
-        message = "ErrorNotifier caught exception: #{exception.message}. Use use ErrorNotifier.expects(:notify) to " \
+        message = "ErrorNotifier caught exception: #{exception.message}. Use ErrorNotifier.expects(:notify) to " \
           "silence in tests"
         raise RuntimeError, message, exception.backtrace
       else

lib/samson/periodical.rb

@@ -8,7 +8,6 @@
 module Samson
   module Periodical
     TASK_DEFAULTS = {
-      now: true, # see TimerTask, run at startup so we are in a consistent and clean state after a restart
       execution_interval: 60, # see TimerTask
       timeout_interval: 10, # see TimerTask
       active: false
@@ -46,11 +45,21 @@ def overdue?(name, since)
       def run
         registered.map do |name, config|
           next unless config.fetch(:active)
+
+          # run at startup so we are in a consistent and clean state after a restart
+          # not using TimerTask `now` option since then initial constant loading would happen in multiple threads
+          # and we run into fun autoload errors like `LoadError: Unable to autoload constant Job` in development/test
+          unless config[:now]
+            ActiveRecord::Base.connection_pool.with_connection do
+              run_once(name)
+            end
+          end
+
           with_consistent_start_time(config) do
             Concurrent::TimerTask.new(config) do
               track_running_count do
-                ActiveRecord::Base.connection_pool.with_connection do
-                  execute_block(config) if enabled
+                if enabled
+                  ActiveRecord::Base.connection_pool.with_connection { execute_block(config) }
                 end
               end
             end.with_observer(ExceptionReporter.new(name)).execute
@@ -59,15 +68,14 @@ def run
       end
 
       # method to test things out on console / testing
-      # simulates timeout that Concurrent::TimerTask does
+      # simulates timeout that Concurrent::TimerTask does and exception reporting
       def run_once(name)
         config = registered.fetch(name)
         Timeout.timeout(config.fetch(:timeout_interval)) do
           execute_block(config)
         end
       rescue
         ExceptionReporter.new(name).update(nil, nil, $!)
-        raise
       end
 
       def interval(name)

test/lib/error_notifier_test.rb

@@ -24,7 +24,7 @@
         ErrorNotifier.notify(exception)
       end
 
-      expected_message = "ErrorNotifier caught exception: motherofgod. Use use ErrorNotifier.expects(:notify)" \
+      expected_message = "ErrorNotifier caught exception: motherofgod. Use ErrorNotifier.expects(:notify)" \
         " to silence in tests"
       e.message.must_equal expected_message
       e.backtrace.must_equal ['neatbacktraceyougotthere']

test/lib/samson/periodical_test.rb

@@ -80,9 +80,7 @@ def with_registered
       Lock.expects(:remove_expired_locks).raises custom_error
       ErrorNotifier.expects(:notify).
         with(instance_of(custom_error), error_message: "Samson::Periodical remove_expired_locks failed")
-      assert_raises custom_error do
-        Samson::Periodical.run_once(:remove_expired_locks)
-      end
+      Samson::Periodical.run_once(:remove_expired_locks)
     end
   end
 
@@ -111,13 +109,20 @@ def with_registered
       Samson::Periodical.run.must_equal []
     end
 
-    it "sends errors to error notifier XXXX" do
-      ErrorNotifier.expects(:notify).with(instance_of(custom_error), error_message: "Samson::Periodical foo failed")
-      Samson::Periodical.register(:foo, 'bar') { raise custom_error }
+    it "sends errors to error notifier" do
+      ErrorNotifier.expects(:notify).with(instance_of(ArgumentError), error_message: "Samson::Periodical foo failed")
+      Samson::Periodical.register(:foo, 'bar', now: true) { raise ArgumentError }
       tasks = Samson::Periodical.run
       sleep 0.05 # let task execute
       tasks.first.shutdown
     end
+
+    it "does not block server boot when initial run is inline and fails" do
+      ErrorNotifier.expects(:notify).with(instance_of(ArgumentError), error_message: "Samson::Periodical foo failed")
+      Samson::Periodical.register(:foo, 'bar') { raise ArgumentError }
+      tasks = Samson::Periodical.run
+      tasks.first.shutdown
+    end
   end
 
   describe ".configs_from_string" do
@@ -178,7 +183,7 @@ def call(*args)
 
     it 'counts running tasks' do
       mutex = Mutex.new.lock
-      Samson::Periodical.register(:foo, 'bar', active: true) { mutex.lock }
+      Samson::Periodical.register(:foo, 'bar', active: true, now: true) { mutex.lock }
       tasks = Samson::Periodical.run
       sleep 0.01 # Allow task to start
 
@@ -190,8 +195,9 @@ def call(*args)
       tasks.first.shutdown
     end
 
-    it 'counts raising tasks' do
-      Samson::Periodical.register(:foo, 'bar', active: true) { raise }
+    it 'correctly counts down when task raised' do
+      ErrorNotifier.expects(:notify)
+      Samson::Periodical.register(:foo, 'bar', active: true, now: true) { raise }
       tasks = Samson::Periodical.run
       sleep 0.01 # Allow task to finish