Do not use advisory lock on heartbeat in production

Related to #1447

lib/good_job/configuration.rb

@@ -37,6 +37,10 @@ class Configuration
     DEFAULT_ENQUEUE_AFTER_TRANSACTION_COMMIT = false
     # Default smaller_number_is_higher_priority
     DEFAULT_SMALLER_NUMBER_IS_HIGHER_PRIORITY = true
+    # Default advisory lock heartbeat in development environment
+    DEFAULT_ADVISORY_LOCK_HEARTBEAT_DEVELOPMENT = true
+    # Default advisory lock heartbeat in other environments
+    DEFAULT_ADVISORY_LOCK_HEARTBEAT_OTHER = false
 
     def self.validate_execution_mode(execution_mode)
       raise ArgumentError, "GoodJob execution mode must be one of #{EXECUTION_MODES.join(', ')}. It was '#{execution_mode}' which is not valid." unless execution_mode.in?(EXECUTION_MODES)
@@ -385,6 +389,16 @@ def in_webserver?
       end || false
     end
 
+    # Whether to take an advisory lock on the process record in the notifier reactor.
+    # @return [Boolean]
+    def advisory_lock_heartbeat
+      return options[:advisory_lock_heartbeat] unless options[:advisory_lock_heartbeat].nil?
+      return rails_config[:advisory_lock_heartbeat] unless rails_config[:advisory_lock_heartbeat].nil?
+      return ActiveModel::Type::Boolean.new.cast(env['GOOD_JOB_ADVISORY_LOCK_HEARTBEAT']) unless env['GOOD_JOB_ADVISORY_LOCK_HEARTBEAT'].nil?
+
+      Rails.env.development? ? DEFAULT_ADVISORY_LOCK_HEARTBEAT_DEVELOPMENT : DEFAULT_ADVISORY_LOCK_HEARTBEAT_OTHER
+    end
+
     private
 
     def rails_config

lib/good_job/notifier/process_heartbeat.rb

@@ -14,9 +14,10 @@ module ProcessHeartbeat
 
       # Registers the current process.
       def register_process
+        @advisory_lock_heartbeat = GoodJob.configuration.advisory_lock_heartbeat
         GoodJob::Process.override_connection(connection) do
           GoodJob::Process.cleanup
-          @capsule.tracker.register(with_advisory_lock: true)
+          @capsule.tracker.register(with_advisory_lock: @advisory_lock_heartbeat)
         end
       end
 
@@ -33,7 +34,7 @@ def refresh_process
       # Deregisters the current process.
       def deregister_process
         GoodJob::Process.override_connection(connection) do
-          @capsule.tracker.unregister(with_advisory_lock: true)
+          @capsule.tracker.unregister(with_advisory_lock: @advisory_lock_heartbeat)
         end
       end
     end

spec/lib/good_job/configuration_spec.rb

@@ -326,4 +326,35 @@
       expect(configuration.dashboard_live_poll_enabled).to eq true
     end
   end
+
+  describe '#advisory_lock_heartbeat' do
+    it 'defaults to true in development' do
+      allow(Rails).to receive(:env) { "development".inquiry }
+      configuration = described_class.new({})
+      expect(configuration.advisory_lock_heartbeat).to be true
+    end
+
+    it 'defaults to false in other environments' do
+      allow(Rails).to receive(:env) { "production".inquiry }
+      configuration = described_class.new({})
+      expect(configuration.advisory_lock_heartbeat).to be false
+    end
+
+    it 'can be overridden by options' do
+      configuration = described_class.new({ advisory_lock_heartbeat: true })
+      expect(configuration.advisory_lock_heartbeat).to be true
+    end
+
+    it 'can be overridden by rails config' do
+      allow(Rails.application.config).to receive(:good_job).and_return({ advisory_lock_heartbeat: true })
+      configuration = described_class.new({})
+      expect(configuration.advisory_lock_heartbeat).to be true
+    end
+
+    it 'can be overridden by environment variable' do
+      stub_const 'ENV', ENV.to_hash.merge({ 'GOOD_JOB_ADVISORY_LOCK_HEARTBEAT' => 'true' })
+      configuration = described_class.new({})
+      expect(configuration.advisory_lock_heartbeat).to be true
+    end
+  end
 end

spec/lib/good_job/notifier_spec.rb

@@ -204,5 +204,43 @@
       notifier.shutdown
       expect { process.reload }.to raise_error ActiveRecord::RecordNotFound
     end
+
+    context 'when advisory_lock_heartbeat is false' do
+      before do
+        allow(GoodJob.configuration).to receive(:advisory_lock_heartbeat).and_return(false)
+      end
+
+      it 'does not take an advisory lock on the process record' do
+        notifier = described_class.new(enable_listening: true)
+
+        wait_until { expect(GoodJob::Process.count).to eq 1 }
+
+        process = GoodJob::Process.first
+        expect(process.id).to eq GoodJob.capsule.tracker.id_for_lock
+        expect(process).not_to be_advisory_locked
+
+        notifier.shutdown
+        expect { process.reload }.to raise_error ActiveRecord::RecordNotFound
+      end
+    end
+
+    context 'when advisory_lock_heartbeat is true' do
+      before do
+        allow(GoodJob.configuration).to receive(:advisory_lock_heartbeat).and_return(true)
+      end
+
+      it 'takes an advisory lock on the process record' do
+        notifier = described_class.new(enable_listening: true)
+
+        wait_until { expect(GoodJob::Process.count).to eq 1 }
+
+        process = GoodJob::Process.first
+        expect(process.id).to eq GoodJob.capsule.tracker.id_for_lock
+        expect(process).to be_advisory_locked
+
+        notifier.shutdown
+        expect { process.reload }.to raise_error ActiveRecord::RecordNotFound
+      end
+    end
   end
 end