Skip to content

Commit

Permalink
Revert "[webui][api] Change SendEventEmails to 1 event per job."
Browse files Browse the repository at this point in the history
This reverts commit df92a8f.
Reverted because delayed job cannot handle large number of jobs
in the queue. We should first move to an async system which can
scale, and then move to 1 event / job.
  • Loading branch information
Evan Rolfe committed Feb 9, 2018
1 parent 6283cab commit 72d499a
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 27 deletions.
30 changes: 16 additions & 14 deletions src/api/app/jobs/send_event_emails_job.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
class SendEventEmailsJob < ApplicationJob
queue_as :mailers

def perform(event_id)
event = Event::Base.find(event_id)
subscribers = event.subscribers
def perform
Event::Base.where(mails_sent: false).order(created_at: :asc).limit(1000).each do |event|
subscribers = event.subscribers

if subscribers.empty?
event.update_attributes(mails_sent: true)
return
end
if subscribers.empty?
event.update_attributes(mails_sent: true)
next
end

begin
create_rss_notifications(event)
EventMailer.event(subscribers, event).deliver_now
rescue StandardError => e
Airbrake.notify(e, event_id: event.id)
ensure
event.update_attributes(mails_sent: true)
begin
create_rss_notifications(event)
EventMailer.event(subscribers, event).deliver_now
rescue StandardError => e
Airbrake.notify(e, event_id: event.id)
ensure
event.update_attributes(mails_sent: true)
end
end
true
end

private
Expand Down
8 changes: 2 additions & 6 deletions src/api/app/models/event/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ class Base < ApplicationRecord
self.table_name = 'events'

after_create :create_project_log_entry_job, if: -> { (PROJECT_CLASSES | PACKAGE_CLASSES).include?(self.class.name) }
after_create :perform_create_jobs
after_create :send_event_emails_job

EXPLANATION_FOR_NOTIFICATIONS = {
'Event::BuildFail' => 'Receive notifications of build failures for packages for which you are...',
Expand Down Expand Up @@ -153,6 +151,8 @@ def create_project_log_entry_job
CreateProjectLogEntryJob.perform_later(payload, created_at.to_s, self.class.name)
end

after_create :perform_create_jobs

def perform_create_jobs
self.undone_jobs = 0
save
Expand All @@ -170,10 +170,6 @@ def perform_create_jobs
save if self.undone_jobs > 0
end

def send_event_emails_job
SendEventEmailsJob.perform_later(id) if id.present?
end

# to be overwritten in subclasses
def subject
'Build Service Notification'
Expand Down
4 changes: 4 additions & 0 deletions src/api/config/clock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ module Clockwork
WorkerStatus.new.update_workerstatus_cache
end

every(30.seconds, 'send notifications') do
SendEventEmailsJob.perform_later
end

every(49.minutes, 'rescale history') do
StatusHistoryRescalerJob.perform_later
end
Expand Down
14 changes: 7 additions & 7 deletions src/api/spec/jobs/send_event_emails_job_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'rails_helper'

RSpec.describe SendEventEmailsJob, type: :job do
include ActiveJob::TestHelper

describe '#perform' do
before do
ActionMailer::Base.deliveries = []
Expand All @@ -13,18 +15,16 @@
let!(:project) { create(:project, name: 'comment_project', maintainer: [user, group]) }

let!(:comment_author) { create(:confirmed_user) }
let!(:user_maintainer) { create(:group) }

let(:comment) do
create(:comment_project, commentable: project, body: "Hey @#{user.login} how are things?", user: comment_author)
end
let!(:comment) { create(:comment_project, commentable: project, body: "Hey @#{user.login} how are things?", user: comment_author) }
let!(:user_maintainer) { create(:group) }

context 'with no errors being raised' do
let!(:subscription1) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: user) }
let!(:subscription2) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: group) }
let!(:subscription3) { create(:event_subscription_comment_for_project, receiver_role: 'commenter', user: comment_author) }

subject! { comment }
subject! { SendEventEmailsJob.new.perform }

it 'sends an email to the subscribers' do
email = ActionMailer::Base.deliveries.first
Expand Down Expand Up @@ -76,7 +76,7 @@
allow(Airbrake).to receive(:notify)
end

subject! { comment }
subject! { SendEventEmailsJob.new.perform }

it 'updates the event mails_sent = true' do
event = Event::CommentForProject.first
Expand All @@ -89,7 +89,7 @@
end

context 'with no subscriptions for the event' do
subject! { comment }
subject! { SendEventEmailsJob.new.perform }

it 'updates the event mails_sent = true' do
event = Event::CommentForProject.first
Expand Down

0 comments on commit 72d499a

Please sign in to comment.