From 34c5a1e888938b56c25299b70ce7d846840e3e5c Mon Sep 17 00:00:00 2001 From: "Justin Craig-Kuhn (JCK)" Date: Tue, 14 Nov 2023 16:53:08 -0800 Subject: [PATCH] Fix deadlock, remove Honeybadger --- app/controllers/oauth/sorcery_controller.rb | 2 +- app/services/base/subteam_sync_service.rb | 3 +- app/services/event_service.rb | 2 +- app/services/slack/post_service.rb | 2 - app/services/tip_outcome_service.rb | 43 ++++++++++++--------- 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/app/controllers/oauth/sorcery_controller.rb b/app/controllers/oauth/sorcery_controller.rb index 2b649e1..1132109 100644 --- a/app/controllers/oauth/sorcery_controller.rb +++ b/app/controllers/oauth/sorcery_controller.rb @@ -9,7 +9,7 @@ def callback return redirect_back_or_to(dashboard_path) if login_from(params[:provider]) create_user_and_login rescue StandardError => e - Honeybadger.notify(e) if defined?(Honeybadger) + Rails.logger.error(e) redirect_to login_path, alert: t('auth.external_fail', provider: provider_title) end diff --git a/app/services/base/subteam_sync_service.rb b/app/services/base/subteam_sync_service.rb index 9cd89d7..54f2f22 100644 --- a/app/services/base/subteam_sync_service.rb +++ b/app/services/base/subteam_sync_service.rb @@ -36,8 +36,7 @@ def find_or_create_subteam(attrs) # rubocop:disable Metrics/MethodLength Subteam.create!(combined_attrs) end rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique => e - parameters = { attrs: attrs.to_h, combined_attrs: } - Honeybadger.notify(e, parameters:) if defined?(Honeybadger) + Rails.logger.error(e) nil end diff --git a/app/services/event_service.rb b/app/services/event_service.rb index 7f01b76..918617f 100644 --- a/app/services/event_service.rb +++ b/app/services/event_service.rb @@ -5,7 +5,7 @@ def call return post_success_message if respond_in_chat? delete_slack_ack_message if slack_fast_acked? rescue StandardError => e - Honeybadger.notify(e) if defined?(Honeybadger) && reportable?(e) + Rails.logger.error(e) if reportable?(e) post_error_message(e) end diff --git a/app/services/slack/post_service.rb b/app/services/slack/post_service.rb index 8d33447..eb87830 100644 --- a/app/services/slack/post_service.rb +++ b/app/services/slack/post_service.rb @@ -124,8 +124,6 @@ def alt_text def respond_dm(to_profile_rid) post_params = message_params(to_profile_rid).merge(as_user: true) slack_client.chat_postMessage(post_params) - rescue Slack::Web::Api::Errors::InvalidBlocks => e - Honeybadger.notify(e, parameters: post_params) end def respond_ephemeral(to_profile_rid) diff --git a/app/services/tip_outcome_service.rb b/app/services/tip_outcome_service.rb index 07797e9..c52eeb0 100644 --- a/app/services/tip_outcome_service.rb +++ b/app/services/tip_outcome_service.rb @@ -18,6 +18,8 @@ def call def update_profile_and_team_stats Tip.transaction do + lock_records_for_transaction + update_to_profiles update_from_profile update_team @@ -25,36 +27,39 @@ def update_profile_and_team_stats end end + # Lock profiles in a consistent order to reduce deadlocks + def lock_records_for_transaction + [team, from_profile, *to_profiles].sort_by(&:id).each(&:lock!) + end + + def to_profiles + @to_profiles ||= tips.filter_map(&:to_profile).uniq + end + # rubocop:disable Metrics/AbcSize def update_to_profiles tips.each do |tip| profile = tip.to_profile - profile.with_lock do - last_tip_received_at = destroy ? previous_received_at(profile) : tip.created_at - value_col = tip.jab? ? :jabs_received : :points_received - value = profile.send(value_col).send(operator, tip.quantity.abs) - balance = profile.balance.send(operator, tip.quantity) - profile.update!(value_col => value, balance:, last_tip_received_at:) - end + last_tip_received_at = destroy ? previous_received_at(profile) : tip.created_at + value_col = tip.jab? ? :jabs_received : :points_received + value = profile.send(value_col).send(operator, tip.quantity.abs) + balance = profile.balance.send(operator, tip.quantity) + profile.update!(value_col => value, balance:, last_tip_received_at:) end end def update_from_profile - from_profile.with_lock do - points_sent = from_profile.points_sent.send(operator, total_points) - jabs_sent = from_profile.jabs_sent.send(operator, total_jabs) - last_tip_sent_at = destroy ? previous_sent_at : tips.first.created_at - from_profile.update!(points_sent:, jabs_sent:, last_tip_sent_at:) - end + points_sent = from_profile.points_sent.send(operator, total_points) + jabs_sent = from_profile.jabs_sent.send(operator, total_jabs) + last_tip_sent_at = destroy ? previous_sent_at : tips.first.created_at + from_profile.update!(points_sent:, jabs_sent:, last_tip_sent_at:) end def update_team - team.with_lock do - points_sent = team.points_sent.send(operator, total_points) - jabs_sent = team.jabs_sent.send(operator, total_jabs) - balance = team.balance.send(operator, total_points - total_jabs) - team.update!(points_sent:, jabs_sent:, balance:) - end + points_sent = team.points_sent.send(operator, total_points) + jabs_sent = team.jabs_sent.send(operator, total_jabs) + balance = team.balance.send(operator, total_points - total_jabs) + team.update!(points_sent:, jabs_sent:, balance:) end # rubocop:enable Metrics/AbcSize