From f646cf500aef1d37aeb28a4a11f5bf2a4930837c Mon Sep 17 00:00:00 2001 From: Stephen Crosby Date: Sun, 14 Jun 2015 16:26:52 -0700 Subject: [PATCH] reduce number of queries on insert --- app/models/app.rb | 6 ++++-- app/models/backtrace.rb | 13 +++++++------ app/models/error_report.rb | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- app/models/notice.rb | 51 +-------------------------------------------------- app/models/problem.rb | 10 +--------- 5 files changed, 81 insertions(+), 71 deletions(-) diff --git a/app/models/app.rb b/app/models/app.rb index fea8305..338b9e3 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -57,8 +57,10 @@ class App def find_or_create_err!(attrs) Err.where( :fingerprint => attrs[:fingerprint] - ).first || - problems.create!(attrs.slice(:error_class, :environment)).errs.create!(attrs.slice(:fingerprint, :problem_id)) + ).first || ( + problem = problems.create!(attrs.slice(:error_class, :environment)) + problem.errs.create!(attrs.slice(:fingerprint, :problem_id)) + ) end # Mongoid Bug: find(id) on association proxies returns an Enumerator diff --git a/app/models/backtrace.rb b/app/models/backtrace.rb index 86f0453..113358c 100644 --- a/app/models/backtrace.rb +++ b/app/models/backtrace.rb @@ -14,12 +14,13 @@ class Backtrace delegate :app, :to => :notice - def self.find_or_create(attributes = {}) - new(attributes).similar.find_and_modify({ '$setOnInsert' => attributes }) - end + def self.find_or_create(lines) + fingerprint = generate_fingerprint(lines) - def similar - Backtrace.where(:fingerprint => fingerprint) + where(fingerprint: generate_fingerprint(lines)). + find_one_and_update( + { '$setOnInsert' => { fingerprint: fingerprint, lines: lines } }, + { return_document: :after, upsert: true }) end def raw=(raw) @@ -30,6 +31,6 @@ class Backtrace private def generate_fingerprint - self.fingerprint = Digest::SHA1.hexdigest(lines.map(&:to_s).join) + self.fingerprint = self.class.generate_fingerprint(lines) end end diff --git a/app/models/error_report.rb b/app/models/error_report.rb index e3c4868..bbfe1f6 100644 --- a/app/models/error_report.rb +++ b/app/models/error_report.rb @@ -40,24 +40,88 @@ class ErrorReport end def backtrace - @normalized_backtrace ||= Backtrace.find_or_create(raw: @backtrace) + @normalized_backtrace ||= Backtrace.find_or_create(@backtrace) end def generate_notice! return unless valid? return @notice if @notice + + make_notice + error.notices << @notice + cache_attributes_on_problem + email_notification + services_notification + @notice + end + + def make_notice @notice = Notice.new( message: message, error_class: error_class, - backtrace_id: backtrace.id, + backtrace: backtrace, request: request, server_environment: server_environment, notifier: notifier, user_attributes: user_attributes, framework: framework ) - error.notices << @notice - @notice + end + + # Update problem cache with information about this notice + def cache_attributes_on_problem + # increment notice count + message_digest = Digest::MD5.hexdigest(@notice.message) + host_digest = Digest::MD5.hexdigest(@notice.host) + user_agent_digest = Digest::MD5.hexdigest(@notice.user_agent_string) + + @problem = Problem.where("_id" => @error.problem_id).find_one_and_update( + '$set' => { + 'app_name' => app.name, + 'environment' => @notice.environment_name, + 'error_class' => @notice.error_class, + 'last_notice_at' => @notice.created_at, + 'message' => @notice.message, + 'resolved' => false, + 'resolved_at' => nil, + 'where' => @notice.where, + "messages.#{message_digest}.value" => @notice.message, + "hosts.#{host_digest}.value" => @notice.host, + "user_agents.#{user_agent_digest}.value" => @notice.user_agent_string, + }, + '$inc' => { + 'notices_count' => 1, + "messages.#{message_digest}.count" => 1, + "hosts.#{host_digest}.count" => 1, + "user_agents.#{user_agent_digest}.count" => 1, + } + ) + end + + def similar_count + @similar_count ||= @problem.notices_count + end + + # Send email notification if needed + def email_notification + return false unless app.emailable? + return false unless app.email_at_notices.include?(similar_count) + Mailer.err_notification(@notice).deliver + rescue => e + HoptoadNotifier.notify(e) + end + + def should_notify? + app.notification_service.notify_at_notices.include?(0) || + app.notification_service.notify_at_notices.include?(similar_count) + end + + # Launch all notification define on the app associate to this notice + def services_notification + return true unless app.notification_service_configured? and should_notify? + app.notification_service.create_notification(problem) + rescue => e + HoptoadNotifier.notify(e) end ## diff --git a/app/models/notice.rb b/app/models/notice.rb index 5be37d7..78e1ace 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -20,13 +20,10 @@ class Notice index(:created_at => 1) index(:err_id => 1, :created_at => 1, :_id => 1) - after_create :cache_attributes_on_problem, :unresolve_problem - after_create :email_notification - after_create :services_notification before_save :sanitize before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem - validates_presence_of :backtrace, :server_environment, :notifier + validates_presence_of :backtrace_id, :server_environment, :notifier scope :ordered, ->{ order_by(:created_at.asc) } scope :reverse_ordered, ->{ order_by(:created_at.desc) } @@ -110,22 +107,6 @@ class Notice backtrace_lines.in_app end - def similar_count - problem.notices_count - end - - def emailable? - app.email_at_notices.include?(similar_count) - end - - def should_email? - app.emailable? && emailable? - end - - def should_notify? - app.notification_service.notify_at_notices.include?(0) || app.notification_service.notify_at_notices.include?(similar_count) - end - ## # TODO: Move on decorator maybe # @@ -151,21 +132,12 @@ class Notice problem.remove_cached_notice_attributes(self) if err end - def unresolve_problem - problem.update_attributes!(:resolved => false, :resolved_at => nil, :notices_count => 1) if problem.resolved? - end - - def cache_attributes_on_problem - ProblemUpdaterCache.new(problem, self).update - end - def sanitize [:server_environment, :request, :notifier].each do |h| send("#{h}=",sanitize_hash(send(h))) end end - def sanitize_hash(h) h.recurse do |h| h.inject({}) do |h,(k,v)| @@ -178,25 +150,4 @@ class Notice end end end - - private - - ## - # Send email notification if needed - def email_notification - return true unless should_email? - Mailer.err_notification(self).deliver - rescue => e - HoptoadNotifier.notify(e) - end - - ## - # Launch all notification define on the app associate to this notice - def services_notification - return true unless app.notification_service_configured? and should_notify? - app.notification_service.create_notification(problem) - rescue => e - HoptoadNotifier.notify(e) - end - end diff --git a/app/models/problem.rb b/app/models/problem.rb index f9a61b7..455d895 100644 --- a/app/models/problem.rb +++ b/app/models/problem.rb @@ -132,15 +132,7 @@ class Problem def cache_app_attributes if app self.app_name = app.name - self.last_deploy_at = if (last_deploy = app.deploys.where(:environment => self.environment).last) - last_deploy.created_at.utc - end - collection. - find('_id' => self.id). - update_one({'$set' => { - 'app_name' => self.app_name, - 'last_deploy_at' => self.last_deploy_at.try(:utc) - }}) + self.last_deploy_at = app.last_deploy_at end end -- libgit2 0.21.2