Fix race condition in Card::ActivitySpike::DetectionJob
The check-then-act pattern in `register_activity_spike` has been replaced with `find_or_create_by!` to eliminate the race condition that could lead to creating multiple activity spikes for a card. To support this change, the `card_id` index on `cards_activity_spikes` has been made unique. ref: https://app.fizzy.do/5986089/cards/3063
This commit is contained in:
@@ -21,11 +21,7 @@ class Card::ActivitySpike::Detector
|
||||
|
||||
def register_activity_spike
|
||||
Card.suppressing_turbo_broadcasts do
|
||||
if card.activity_spike
|
||||
card.activity_spike.touch
|
||||
else
|
||||
card.create_activity_spike!
|
||||
end
|
||||
Card::ActivitySpike.find_or_create_by!(card: card).touch
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
class AddUniqueIndexToCardActivitySpikesOnCardId < ActiveRecord::Migration[8.2]
|
||||
def change
|
||||
reversible do |dir|
|
||||
dir.up do
|
||||
execute <<-SQL
|
||||
DELETE s1 FROM card_activity_spikes s1
|
||||
INNER JOIN card_activity_spikes s2
|
||||
WHERE s1.card_id = s2.card_id
|
||||
AND s1.updated_at < s2.updated_at
|
||||
SQL
|
||||
end
|
||||
end
|
||||
|
||||
remove_index :card_activity_spikes, :card_id
|
||||
add_index :card_activity_spikes, :card_id, unique: true
|
||||
end
|
||||
end
|
||||
Generated
+2
-2
@@ -10,7 +10,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema[8.2].define(version: 2025_11_20_194700) do
|
||||
ActiveRecord::Schema[8.2].define(version: 2025_11_20_203100) do
|
||||
create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
|
||||
t.datetime "accessed_at"
|
||||
t.uuid "account_id", null: false
|
||||
@@ -150,7 +150,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_20_194700) do
|
||||
t.datetime "created_at", null: false
|
||||
t.datetime "updated_at", null: false
|
||||
t.index ["account_id"], name: "index_card_activity_spikes_on_account_id"
|
||||
t.index ["card_id"], name: "index_card_activity_spikes_on_card_id"
|
||||
t.index ["card_id"], name: "index_card_activity_spikes_on_card_id", unique: true
|
||||
end
|
||||
|
||||
create_table "card_engagements", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
|
||||
|
||||
@@ -39,6 +39,21 @@ class Card::ActivitySpike::DetectorTest < ActiveSupport::TestCase
|
||||
assert @card.reload.activity_spike.updated_at > original_last_spike_at
|
||||
end
|
||||
|
||||
test "concurrent spike creation should not create multiple spikes for a card" do
|
||||
multiple_people_comment_on(@card)
|
||||
@card.activity_spike&.destroy
|
||||
|
||||
5.times.map do
|
||||
Thread.new do
|
||||
ActiveRecord::Base.connection_pool.with_connection do
|
||||
Card.find(@card.id).detect_activity_spikes
|
||||
end
|
||||
end
|
||||
end.each(&:join)
|
||||
|
||||
assert_equal 1, Card::ActivitySpike.where(card: @card).count
|
||||
end
|
||||
|
||||
private
|
||||
def assert_activity_spike_detected(card: @card)
|
||||
assert card.activity_spike.blank?
|
||||
|
||||
Reference in New Issue
Block a user