From afff1479e85c3effd9157731dd0e36f4d3bf8876 Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Mon, 16 Sep 2024 12:22:15 +0200
Subject: [PATCH] Clear previous changes before locking process for heartbeat

For example, in case of a previous heartbeat failed because
of a DB issue (with SQLite depending on configuration, a
`BusyException` is not rare) and we still have the unpersisted
value in `last_heartbeat_at`, which means that `with_lock` will result
in:
```
RuntimeError: Locking a record with unpersisted changes is not supported
```

Fixes #350
---
 app/models/solid_queue/process.rb        |  6 +++++-
 lib/solid_queue/processes/registrable.rb |  2 +-
 test/models/solid_queue/process_test.rb  | 12 ++++++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/app/models/solid_queue/process.rb b/app/models/solid_queue/process.rb
index 4ab4bbb0..e81edc14 100644
--- a/app/models/solid_queue/process.rb
+++ b/app/models/solid_queue/process.rb
@@ -20,7 +20,11 @@ def self.register(**attributes)
   end
 
   def heartbeat
-    touch(:last_heartbeat_at)
+    # Clear any previous changes before locking, for example, in case a previous heartbeat
+    # failed because of a DB issue (with SQLite depending on configuration, a BusyException
+    # is not rare) and we still have the unpersisted value
+    restore_attributes
+    with_lock { touch(:last_heartbeat_at) }
   end
 
   def deregister(pruned: false)
diff --git a/lib/solid_queue/processes/registrable.rb b/lib/solid_queue/processes/registrable.rb
index 24941238..58cabfa8 100644
--- a/lib/solid_queue/processes/registrable.rb
+++ b/lib/solid_queue/processes/registrable.rb
@@ -53,7 +53,7 @@ def stop_heartbeat
       end
 
       def heartbeat
-        process.with_lock { process.heartbeat }
+        process.heartbeat
       rescue ActiveRecord::RecordNotFound
         self.process = nil
         wake_up
diff --git a/test/models/solid_queue/process_test.rb b/test/models/solid_queue/process_test.rb
index 61343de8..5504f0e6 100644
--- a/test/models/solid_queue/process_test.rb
+++ b/test/models/solid_queue/process_test.rb
@@ -69,4 +69,16 @@ class SolidQueue::ProcessTest < ActiveSupport::TestCase
       worker.stop
     end
   end
+
+  test "clear unregistered changes before locking for heartbeat" do
+    process = SolidQueue::Process.register(kind: "Supervisor", pid: 43, name: "supervisor-43")
+
+    # Pretend a heartbeat failed to persist
+    failed_heartbeat_at = 10.minutes.ago
+    process.last_heartbeat_at = failed_heartbeat_at
+
+    assert_changes -> { process.last_heartbeat_at } do
+      process.heartbeat
+    end
+  end
 end