Merge pull request #3951 from evazion/fix-maintenance-logging

Improve daily maintenance error handling

app/logical/daily_maintenance.rb

@@ -1,21 +0,0 @@
-class DailyMaintenance
-  def run
-    ActiveRecord::Base.connection.execute("set statement_timeout = 0")
-    PostPruner.new.prune!
-    Upload.where('created_at < ?', 1.day.ago).delete_all
-    Delayed::Job.where('created_at < ?', 45.days.ago).delete_all
-    PostVote.prune!
-    CommentVote.prune!
-    ApiCacheGenerator.new.generate_tag_cache
-    PostDisapproval.prune!
-    ForumSubscription.process_all!
-    TagAlias.update_cached_post_counts_for_all
-    PostDisapproval.dmail_messages!
-    Tag.clean_up_negative_post_counts!
-    SuperVoter.init!
-    TokenBucket.prune!
-    TagChangeRequestPruner.warn_all
-    TagChangeRequestPruner.reject_all
-    Ban.prune!
-  end
-end

app/logical/maintenance.rb

@@ -0,0 +1,52 @@
+module Maintenance
+  module_function
+
+  def hourly
+    UploadErrorChecker.new.check!
+    DelayedJobErrorChecker.new.check!
+  rescue Exception => exception
+    rescue_exception(exception)
+  end
+
+  def daily
+    ActiveRecord::Base.connection.execute("set statement_timeout = 0")
+    PostPruner.new.prune!
+    Upload.where('created_at < ?', 1.day.ago).delete_all
+    Delayed::Job.where('created_at < ?', 45.days.ago).delete_all
+    PostVote.prune!
+    CommentVote.prune!
+    ApiCacheGenerator.new.generate_tag_cache
+    PostDisapproval.prune!
+    ForumSubscription.process_all!
+    TagAlias.update_cached_post_counts_for_all
+    PostDisapproval.dmail_messages!
+    Tag.clean_up_negative_post_counts!
+    SuperVoter.init!
+    TokenBucket.prune!
+    TagChangeRequestPruner.warn_all
+    TagChangeRequestPruner.reject_all
+    Ban.prune!
+  rescue Exception => exception
+    rescue_exception(exception)
+  end
+
+  def weekly
+    ActiveRecord::Base.connection.execute("set statement_timeout = 0")
+    UserPasswordResetNonce.prune!
+    ApproverPruner.prune!
+    TagRelationshipRetirementService.find_and_retire!
+  rescue Exception => exception
+    rescue_exception(exception)
+  end
+
+  def rescue_exception(exception)
+    backtrace = Rails.backtrace_cleaner.clean(exception.backtrace).join("\n")
+    Rails.logger.error("#{exception.class}: #{exception.message}\n#{backtrace}")
+
+    if defined?(NewRelic::Agent)
+      NewRelic::Agent.notice_error(exception, custom_params: { backtrace: backtrace })
+    end
+
+    raise exception
+  end
+end

app/logical/monthly_maintenance.rb

@@ -1,5 +0,0 @@
-class MonthlyMaintenance
-  def run
-    ActiveRecord::Base.connection.execute("set statement_timeout = 0")
-  end
-end

app/logical/weekly_maintenance.rb

@@ -1,8 +0,0 @@
-class WeeklyMaintenance
-  def run
-    ActiveRecord::Base.connection.execute("set statement_timeout = 0")
-    UserPasswordResetNonce.prune!
-    ApproverPruner.prune!
-    # JanitorPruner.new.prune!
-  end
-end

config/schedule.rb

@@ -2,15 +2,11 @@ set :output, "/var/log/whenever.log"
 #env "MAILTO", "webmaster@danbooru.donmai.us"
 
 every 1.hour do
-  runner "UploadErrorChecker.new.check!"
-end
-
-every 1.hour do
-  runner "DelayedJobErrorChecker.new.check!"
+  rake "maintenance:hourly"
 end
 
 every 1.day do
-  runner "DailyMaintenance.new.run"
+  rake "maintenance:daily"
 end
 
 every 1.day, :at => "1:00 am" do
@@ -18,10 +14,5 @@ every 1.day, :at => "1:00 am" do
 end
 
 every 1.week, :at => "1:30 am" do
-  runner "WeeklyMaintenance.new.run"
-  runner "TagRelationshipRetirementService.find_and_retire!"
-end
-
-every 1.month, :at => "2:00 am" do
-  runner "MonthlyMaintenance.new.run"
+  rake "maintenance:weekly"
 end

lib/tasks/maintenance.rake

@@ -0,0 +1,18 @@
+require "tasks/newrelic" if defined?(NewRelic)
+
+namespace :maintenance do
+  desc "Run hourly maintenance jobs"
+  task hourly: :environment do
+    Maintenance.hourly
+  end
+
+  desc "Run daily maintenance jobs"
+  task daily: :environment do
+    Maintenance.daily
+  end
+
+  desc "Run weekly maintenance jobs"
+  task weekly: :environment do
+    Maintenance.weekly
+  end
+end

script/install/newrelic.yml

@@ -0,0 +1,46 @@
+# This file configures the New Relic Agent.  New Relic monitors Ruby, Java,
+# .NET, PHP, Python, Node, and Go applications with deep visibility and low
+# overhead.  For more information, visit www.newrelic.com.
+#
+# For full documentation of agent configuration options, please refer to
+# https://docs.newrelic.com/docs/agents/ruby-agent/installation-configuration/ruby-agent-configuration
+
+common: &default_settings
+  # Required license key associated with your New Relic account.
+  license_key: YOUR_LICENCE_KEY_HERE
+
+  # Your application name. Renaming here affects where data displays in New
+  # Relic.  For more details, see https://docs.newrelic.com/docs/apm/new-relic-apm/maintenance/renaming-applications
+  app_name: Danbooru
+
+  # To disable the agent regardless of other settings, uncomment the following:
+  # agent_enabled: false
+
+  # Logging level for log/newrelic_agent.log
+  log_level: info
+
+  # https://docs.newrelic.com/docs/agents/ruby-agent/background-jobs/rake-instrumentation
+  rake:
+    tasks: ["maintenance:.*"]
+
+# Environment-specific settings are in this section.
+# RAILS_ENV or RACK_ENV (as appropriate) is used to determine the environment.
+# If your application has other named environments, configure them here.
+development:
+  <<: *default_settings
+  app_name: Danbooru (Development)
+
+  # New Relic is disabled by default in the development environment. Uncomment to enable.
+  # monitor_mode: true
+
+test:
+  <<: *default_settings
+  # It doesn't make sense to report to New Relic from automated test runs.
+  monitor_mode: false
+
+staging:
+  <<: *default_settings
+  app_name: Danbooru (Staging)
+
+production:
+  <<: *default_settings

test/unit/maintenance_test.rb

@@ -1,6 +1,6 @@
 require 'test_helper'
 
-class DailyMaintenanceTest < ActiveSupport::TestCase
+class MaintenanceTest < ActiveSupport::TestCase
   context "daily maintenance" do
     setup do
       # have ApiCacheGenerator save files to a temp dir.
@@ -17,14 +17,14 @@ class DailyMaintenanceTest < ActiveSupport::TestCase
     end
 
     should "work" do
-      assert_nothing_raised { DailyMaintenance.new.run }
+      assert_nothing_raised { Maintenance.daily }
     end
 
     should "prune expired posts" do
       @pending = FactoryBot.create(:post, is_pending: true, created_at: 4.days.ago)
       @flagged = FactoryBot.create(:post, is_flagged: true, created_at: 4.days.ago)
 
-      DailyMaintenance.new.run
+      Maintenance.daily
 
       assert(true, @pending.reload.is_deleted)
       assert(true, @flagged.reload.is_deleted)
@@ -38,7 +38,7 @@ class DailyMaintenanceTest < ActiveSupport::TestCase
         CurrentUser.as(banner) { FactoryBot.create(:ban, user: user, banner: banner, duration: 1) }
 
         assert_equal(true, user.reload.is_banned)
-        travel_to(2.days.from_now) { DailyMaintenance.new.run }
+        travel_to(2.days.from_now) { Maintenance.daily }
         assert_equal(false, user.reload.is_banned)
       end
     end