Skip to content

Commit 0916866

Browse files
authored
Execute some analytics-related inserts async in postgres (forem#22748)
* Execute some analytics-related inserts async in postgres * Add comment
1 parent ad36df8 commit 0916866

File tree

7 files changed

+62
-9
lines changed

7 files changed

+62
-9
lines changed

app/controllers/billboard_events_controller.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ class BillboardEventsController < ApplicationMetalController
66
def create
77
# Only tracking for logged‐in users at the moment
88
billboard_event_create_params = billboard_event_params.merge(user_id: session_current_user_id)
9-
@billboard_event = BillboardEvent.create(billboard_event_create_params)
9+
@billboard_event = ApplicationRecord.with_synchronous_commit_off do
10+
BillboardEvent.create(billboard_event_create_params)
11+
end
1012

1113
unless ApplicationConfig["DISABLE_BILLBOARD_DATA_UPDATE"] == "yes"
1214
# Enqueue the worker instead of doing the update inline

app/models/application_record.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ def self.with_statement_timeout(duration, connection: self.connection)
6262
connection.execute "SET statement_timeout = #{milliseconds}"
6363
end
6464

65+
# Used for high-volume inserts to prevent unnecessary locking and blocking of other operations
66+
def self.with_synchronous_commit_off
67+
transaction do
68+
connection.execute("SET LOCAL synchronous_commit TO OFF")
69+
yield
70+
end
71+
end
72+
6573
# ActiveRecord's `find_each` method allows you to work with a large collection of records
6674
# in batches, but strictly only orders those batches by IDs in ascending order.
6775
# Any other specified order is either ignored or raises an error (depending on configuration).

app/workers/billboards/track_email_click_worker.rb

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ def perform(bb_param, user_id)
1313
private
1414

1515
def track_billboard
16-
BillboardEvent.create(billboard_id: @bb.to_i,
17-
category: "click",
18-
user_id: @user&.id,
19-
context_type: "email")
20-
update_billboard_counts
16+
ApplicationRecord.with_synchronous_commit_off do
17+
BillboardEvent.create(billboard_id: @bb.to_i,
18+
category: "click",
19+
user_id: @user&.id,
20+
context_type: "email")
21+
update_billboard_counts
22+
end
2123
rescue StandardError => e
2224
Rails.logger.error "Error processing billboard click: #{e.message}"
2325
end

app/workers/emails/send_user_digest_worker.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,15 @@ def perform(user_id, force_send = false)
4343
)
4444
.digest_email.deliver_now
4545

46-
event_params = { user_id: user.id, context_type: "email", category: "impression" }
47-
BillboardEvent.create(event_params.merge(billboard_id: first_billboard.id)) if first_billboard.present?
48-
BillboardEvent.create(event_params.merge(billboard_id: second_billboard.id)) if second_billboard.present?
46+
# Track billboard impressions with relaxed durability — these are
47+
# low-priority analytics writes that don't need synchronous WAL flush.
48+
if first_billboard.present? || second_billboard.present?
49+
event_params = { user_id: user.id, context_type: "email", category: "impression" }
50+
ApplicationRecord.with_synchronous_commit_off do
51+
BillboardEvent.create(event_params.merge(billboard_id: first_billboard.id)) if first_billboard.present?
52+
BillboardEvent.create(event_params.merge(billboard_id: second_billboard.id)) if second_billboard.present?
53+
end
54+
end
4955
rescue StandardError => e
5056
Honeybadger.context({ user_id: user.id, article_ids: articles.map(&:id) })
5157
Honeybadger.notify(e)

spec/workers/emails/batch_custom_send_worker_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@
3232
expect(CustomMailer).to have_received(:with).twice
3333
end
3434

35+
it "does not wrap email delivery in a synchronous_commit_off transaction" do
36+
expect(ApplicationRecord).not_to receive(:with_synchronous_commit_off)
37+
worker.perform(user_ids, subject_line, content, type_of, email_id)
38+
end
39+
3540
it "logs an error and continues if one user raises an exception" do
3641
allow(User).to receive(:find_by).with(id: user.id).and_return(user)
3742
allow(User).to receive(:find_by).with(id: user2.id).and_raise(StandardError.new("Boom!"))

spec/workers/emails/drip_email_worker_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@
111111
).once
112112
end
113113

114+
it "does not wrap email delivery in a synchronous_commit_off transaction" do
115+
expect(ApplicationRecord).not_to receive(:with_synchronous_commit_off)
116+
worker.perform
117+
end
118+
114119
it "sends custom template to users with their own onboarding_subforem_id" do
115120
worker.perform
116121
expect(CustomMailer).to have_received(:with).with(

spec/workers/emails/send_user_digest_worker_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,31 @@
8787
expect(BillboardEvent.where(billboard_id: bb_2.id, category: "impression").size).to be(1)
8888
end
8989

90+
it "still delivers email even if billboard event creation raises" do
91+
create_list(:article, 3, user_id: author.id, public_reactions_count: 20, score: 20, tag_list: [tag.name])
92+
create(:billboard, placement_area: "digest_first", published: true, approved: true)
93+
94+
allow(BillboardEvent).to receive(:create).and_raise(StandardError.new("DB error"))
95+
allow(Honeybadger).to receive(:context)
96+
allow(Honeybadger).to receive(:notify)
97+
98+
worker.perform(user.id)
99+
100+
# Email was already delivered before billboard tracking was attempted
101+
expect(message_delivery).to have_received(:deliver_now)
102+
expect(Honeybadger).to have_received(:notify).with(instance_of(StandardError))
103+
end
104+
105+
it "does not open a transaction when no billboards are present" do
106+
create_list(:article, 3, user_id: author.id, public_reactions_count: 20, score: 20, tag_list: [tag.name])
107+
108+
expect(ApplicationRecord).not_to receive(:with_synchronous_commit_off)
109+
110+
worker.perform(user.id)
111+
112+
expect(message_delivery).to have_received(:deliver_now)
113+
end
114+
90115
context "when there's a preferred paired billboard" do
91116
let!(:bb_1) do
92117
create(

0 commit comments

Comments
 (0)