Skip to content

Commit

Permalink
No longer raise exception when scheduler job fails to run after ten t…
Browse files Browse the repository at this point in the history
…ries, but record the error instead.
  • Loading branch information
Maarten Raaphorst authored and MaartenR committed May 19, 2017
1 parent 2e06269 commit e40772f
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 7 deletions.
17 changes: 16 additions & 1 deletion app/assets/javascripts/app/controllers/monitoring.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Index extends App.ControllerSubContent
events:
'click .js-resetToken': 'resetToken'
'click .js-select': 'selectAll'
'click .js-restartDeadJobs': 'restartDeadJobs'

constructor: ->
super
Expand All @@ -29,7 +30,10 @@ class Index extends App.ControllerSubContent
)

render: =>
@html App.view('monitoring')(data: @data)
@html App.view('monitoring')(
data: @data
job_restart_count: @job_restart_count
)

resetToken: (e) =>
e.preventDefault()
Expand All @@ -42,4 +46,15 @@ class Index extends App.ControllerSubContent
@load()
)

restartDeadJobs: (e) =>
e.preventDefault()
@ajax(
id: 'restart_dead_jobs_request'
type: 'POST'
url: "#{@apiPath}/monitoring/restart_dead_jobs"
success: (data) =>
@job_restart_count = data.job_restart_count
@render()
)

App.Config.set('Monitoring', { prio: 3600, name: 'Monitoring', parent: '#system', target: '#system/monitoring', controller: Index, permission: ['admin.monitoring'] }, 'NavBarAdmin')
9 changes: 9 additions & 0 deletions app/assets/javascripts/app/views/monitoring.jst.eco
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@
<% end %>
<% end %>
</ul>
<% if !_.isEmpty(@data.issues): %>
<button class="btn btn--primary js-restartDeadJobs"><%- @T('Restart Dead Jobs') %></button>
<% if !_.isUndefined(@job_restart_count): %>
<p>
<%- @T('Detected %s dead job(s) available for restart', @job_restart_count) %>
<%- ', restarting...' if @job_restart_count > 0 %>
</p>
<% end %>
<% end %>
</div>

</div>
20 changes: 20 additions & 0 deletions app/controllers/monitoring_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ def health_check
issues.push 'scheduler not running'
end

Scheduler.where(status: 'error').each { |scheduler|
issues.push "Failed to run scheduled job \'#{scheduler.name}\'. Cause: #{scheduler.error_message}"
}

token = Setting.get('monitoring_token')

if issues.empty?
Expand Down Expand Up @@ -173,6 +177,22 @@ def token
render json: result, status: :created
end

def restart_dead_jobs
access_check

count = 0
Scheduler.where(status: 'error').where(active: false).each { |scheduler|
scheduler.active = true
scheduler.save
count += 1
}

result = {
job_restart_count: count
}
render json: result
end

private

def token_or_permission_check
Expand Down
15 changes: 12 additions & 3 deletions app/models/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,10 @@ def self.start_job(job)
end

def self._start_job(job, try_count = 0, try_run_time = Time.zone.now)
job.last_run = Time.zone.now
job.pid = Thread.current.object_id
job.last_run = Time.zone.now
job.pid = Thread.current.object_id
job.status = 'ok'
job.error_message = ''
job.save
logger.info "execute #{job.method} (try_count #{try_count})..."
eval job.method() # rubocop:disable Lint/Eval
Expand All @@ -197,7 +199,14 @@ def self._start_job(job, try_count = 0, try_run_time = Time.zone.now)
if try_run_max > try_count
_start_job(job, try_count, try_run_time)
else
raise "STOP thread for #{job.method} after #{try_count} tries (#{e.inspect})"
@@jobs_started[ job.id ] = false
error = "Failed to run #{job.method} after #{try_count} tries #{e.inspect}"
logger.error error

job.error_message = error
job.status = 'error'
job.active = false
job.save
end
end

Expand Down
7 changes: 4 additions & 3 deletions config/routes/monitoring.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
Zammad::Application.routes.draw do
api_path = Rails.configuration.api_path

match api_path + '/monitoring/health_check', to: 'monitoring#health_check', via: :get
match api_path + '/monitoring/status', to: 'monitoring#status', via: :get
match api_path + '/monitoring/token', to: 'monitoring#token', via: :post
match api_path + '/monitoring/health_check', to: 'monitoring#health_check', via: :get
match api_path + '/monitoring/status', to: 'monitoring#status', via: :get
match api_path + '/monitoring/token', to: 'monitoring#token', via: :post
match api_path + '/monitoring/restart_dead_jobs', to: 'monitoring#restart_dead_jobs', via: :post

end
8 changes: 8 additions & 0 deletions db/migrate/20170515000001_scheduler_status.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class SchedulerStatus < ActiveRecord::Migration
def up
change_table :schedulers do |t|
t.string :error_message
t.string :status
end
end
end

0 comments on commit e40772f

Please sign in to comment.