# Triggering the execution of a job at the end of a timed campaign

I have a task that checks for campaigns that are supposed to be finished by checking the end_at date. If finished, then set the status of the campaign to finished, and send the owner of the campaign an email.

The way I see it, is approach 1 and 2 may have the worker queued more than once in the worker pool.

Approach 3, always makes sure that there is only one instance running at a time. Approach 3 also means making sure the worker is run once on every deploy.

I have 3 scenarios to consider implementing this:

APPROACH #1 - Run the cron job. Cron job determines which campaigns the worker should process

# CRON via whenever gem that runs every 3 minutes
campaigns = Campaign.where("properties -> 'end_at' <= ? AND status = ?", Time.now.utc, 'running')

campaigns.each do |campaign|
FinishCampaignWorker.perform_async(campaign.id)
end

# FinishCampaignWorker class
class FinishCampaignWorker
include Sidekiq::Worker

sidekiq_options({
retry: false,
backtrace: true
})

def perform(campaign_id)
begin
campaign = Campaign.unscoped.find(campaign_id)

# Do not update campaign if deleted or deactivated
if campaign.present? && !%w(deleted deactivated).include?(campaign.status)
campaign.status = 'finished'

campaign.save
end
rescue
Airbrake.notify(
error_class:      'Finish campaign worker',
error_message:    "#{$!}", environment_name: Rails.env ) end end end  APPROACH #2 - Cron triggers the worker to run every 3 minutes. Let the worker handle most of the logic # CRON via whenever gem that runs every 3 minutes FinishCampaignWorker.perform_async # FinishCampaignWorker class class FinishCampaignWorker include Sidekiq::Worker sidekiq_options({ retry: false, backtrace: true }) def perform begin campaigns = Campaign.where("properties -> 'end_at' <= ? AND status = ?", Time.now.utc, 'running') campaigns.each do |campaign| campaign = Campaign.unscoped.find(campaign_id) # Do not update campaign if deleted or deactivated if !%w(deleted deactivated).include?(campaign.status) campaign.status = 'finished' UserMailer.post_campaign_notification(campaign.id, campaign.user.id).deliver campaign.save end end rescue Airbrake.notify( error_class: 'Finish campaign worker', error_message: "#{$!}",
environment_name: Rails.env
)
end
end
end


APPROACH #3 - Run the worker from console/chef/other on first deployment, let the worker handle most of the logic, and then reschedule the worker 3 minutes after completion

# FinishCampaignWorker class
class FinishCampaignWorker
include Sidekiq::Worker

sidekiq_options({
retry: false,
backtrace: true
})

def perform
begin
campaigns = Campaign.where("properties -> 'end_at' <= ? AND status = ?", Time.now.utc, 'running')

campaigns.each do |campaign|
campaign = Campaign.unscoped.find(campaign_id)

# Do not update campaign if deleted or deactivated
if !%w(deleted deactivated).include?(campaign.status)
campaign.status = 'finished'

campaign.save
end
end
rescue
Airbrake.notify(
error_class:      'Finish campaign worker',
error_message:    "#{\$!}",
environment_name: Rails.env
)
end

FinishCampaignWorker.perform_at(3.minutes.from_now)
end
end


Your problem is an integration and management problem, rather than a programming problem (though there are some issues there too).

## Approach 3

Approach-3 is a problem, and I would not use it for your task. Repetitive tasks require a well designed and reliable infrastructure to handle them. The quirks of time are such that things like daylight-saving, system clock resets, and other issues will play havoc with things. For example, What will your task do when you 'spring forward' in daylight savings... will it fail because the next scheduled time never happens? In fall, will it go idle for over an hour when the same hour is repeated?

Cron (and on many Linux distributions, Anacron) has very well documented practices and behaviours for these types of issues, and the predictable, and reasonable use case makes a lot of sense. Use Cron.

Additionally, if there is an issue sending a mail, or anything requiring rescue in Approach 3, you will fail to reschedule, and your program will break.

## Approach 2

This is actually quite similar to Approach 1, but it just shifts more logic in to the process method, and it mixes up the responsibilities in that method. SOLID principles suggest having just a single responsibility in each method, so I don't like this approach. Additionally, you can only process one campaign at a time using this approach.

Too much happening in one place, and possible opportunities for parallelism are removed.

## Approach 1

I prefer this approach for the relative simplicity of the single-campaign method, and the way the campaigns are processed.

# Improvements

Having said that, there are problems still. The biggest is that your program is not defensive enough. Even though you expect only one instance of the program to be running at any one time, you should still program defensively and assume there could be more.

Cron (anacron) will ensure that it does not start a second instance of the program if the previous instance has not yet completed, so cron will not cause a problem, but you may have some mix up happening in your testing, or you may run it yourself when cron has it running already, leading to problems from there.

I am not very familiar with RoR, but the following looks like it could lead to conflicts and race conditions with other running processes:

  # Do not update campaign if deleted or deactivated
if campaign.present? && !%w(deleted deactivated).include?(campaign.status)
campaign.status = 'finished'

campaign.save
end


Posting the campaign e-mail could be a slow task and (even if it was fast) there is the possibility that another instance of the program may be processing the same campaign at the same time. If you are using a database for your campaigns you should consider a different approach. I recommend:

• a transactional approach which checks to see whether the campaign is state='finished', updates it to finished, and returns the previous state, and then commits the transaction.
• this guarantees the state will be marked as finished in a single 'atomic' operation
• it saves the Campaign state before sending the mail.
• there is only a short period of time while the campaign is 'locked'
• if the previous state was not 'finished' then send the mail.

At worst case, now, the only risk is that the mail fails to send. That should be logged in a reliable way that allows you to (manually?) identify, resolve, and resend the mails.

Note that a failed mail will fail whether you mark the Campaign complete or not, and repeatedly resending the mail will not make it suddenly work. There is no reason to save the campaign after the mail-send.

• Doesn't Ruby Time handle DST? rxr.whitequark.org/mri/source/time.c#3655 In this case doing something like 1.hour.from_now takes DST into account. Correct me if I am wrong – Christian Fazzini Dec 10 '14 at 16:02
• "if there is an issue sending a mail, or anything requiring rescue in Approach 3, you will fail to reschedule, and your program will break." If the rescue block is triggered (due to any problem), then the consequences are the same for all approaches (i.e. the jobs will never get processed correctly, even if the Cron job triggers the workers. for #1 and #2). I don't think approach #3 will have any special treatment in this regard. – Christian Fazzini Dec 10 '14 at 16:06
• @ChristianFazzini - in approach 3, if the mail fails, then there will be no reschedule. In 1 and 2, the reschedule is handled by cron. – rolfl Dec 10 '14 at 16:07
• Ok that makes sense. I missed that part. One other approach I forgot to mention that I could apply for #1, is to create a mutex block to prevent multiple instances of the worker from running. What are your thoughts on this? – Christian Fazzini Dec 10 '14 at 16:11
• A mutex block would have to ock on something outside the code. It could work, but, in my experience, the better option is to allow parallel execution, and to handle it gracefully. This allows the best of both worlds. This allows, for example, having multiple hosts running against a single central database, etc. – rolfl Dec 10 '14 at 16:14