Merge branch 'catch-timeout-update-max-seats-worker' into 'master'

Catch Timeouts in UpdateMaxSeatsUsedForGitlabComSubscriptionsWorker

See merge request gitlab-org/gitlab!61741

ee/app/workers/update_max_seats_used_for_gitlab_com_subscriptions_worker.rb

@@ -21,6 +21,8 @@ class UpdateMaxSeatsUsedForGitlabComSubscriptionsWorker # rubocop:disable Scalab
         subscription.refresh_seat_attributes!
 
         tuples << [subscription.id, subscription.max_seats_used, subscription.seats_in_use, subscription.seats_owed]
+      rescue ActiveRecord::QueryCanceled => e
+        track_error(e, subscription)
       end
 
       if tuples.present?
@@ -42,9 +44,9 @@ class UpdateMaxSeatsUsedForGitlabComSubscriptionsWorker # rubocop:disable Scalab
 
   private
 
-  def track_error(subscription)
+  def track_error(error, subscription)
     Gitlab::ErrorTracking.track_exception(
-      StandardError.new('Namespace absent'),
+      error,
       gitlab_subscription_id: subscription.id,
       namespace_id: subscription.namespace_id
     )

ee/spec/workers/update_max_seats_used_for_gitlab_com_subscriptions_worker_spec.rb

@@ -3,95 +3,133 @@
 require 'spec_helper'
 
 RSpec.describe UpdateMaxSeatsUsedForGitlabComSubscriptionsWorker do
-  subject { described_class.new }
+  describe '#perform' do
+    subject { described_class.new }
 
-  let_it_be(:bronze_plan) { create(:bronze_plan) }
-  let_it_be(:gitlab_subscription, refind: true) { create(:gitlab_subscription, seats: 1) }
-  let_it_be(:gitlab_subscription_2, refind: true) { create(:gitlab_subscription, seats: 11) }
+    let_it_be(:bronze_plan) { create(:bronze_plan) }
+    let_it_be(:gitlab_subscription, refind: true) { create(:gitlab_subscription, seats: 1) }
+    let_it_be(:gitlab_subscription_2, refind: true) { create(:gitlab_subscription, seats: 11) }
 
-  let(:db_is_read_only) { false }
-  let(:subscription_attrs) { nil }
+    let(:db_is_read_only) { false }
+    let(:subscription_attrs) { nil }
 
-  before do
-    allow(Gitlab::Database).to receive(:read_only?) { db_is_read_only }
-    allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) { true }
-
-    allow_next_found_instance_of(GitlabSubscription) do |subscription|
-      allow(subscription).to receive(:refresh_seat_attributes!) do
-        subscription.max_seats_used = subscription.seats + 3
-        subscription.seats_in_use = subscription.seats + 2
-        subscription.seats_owed = subscription.seats + 1
-      end
+    before do
+      allow(Gitlab::Database).to receive(:read_only?) { db_is_read_only }
+      allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) { true }
     end
-  end
 
-  def perform_and_reload
-    subject.perform
-    gitlab_subscription.reload
-    gitlab_subscription_2.reload
-  end
-
-  shared_examples 'updates nothing' do
-    it 'does not update seat columns' do
-      expect do
-        perform_and_reload
-      end.to not_change(gitlab_subscription, :max_seats_used)
-        .and not_change(gitlab_subscription, :seats_in_use)
-        .and not_change(gitlab_subscription, :seats_owed)
-        .and not_change(gitlab_subscription_2, :max_seats_used)
-        .and not_change(gitlab_subscription_2, :seats_in_use)
-        .and not_change(gitlab_subscription_2, :seats_owed)
+    def perform_and_reload
+      subject.perform
+      gitlab_subscription.reload
+      gitlab_subscription_2.reload
     end
-  end
 
-  shared_examples 'updates only paid plans' do
-    it "persists seat attributes after refresh_seat_attributes! for only paid plans" do
-      expect do
-        perform_and_reload
-      end.to not_change(gitlab_subscription, :max_seats_used)
-        .and not_change(gitlab_subscription, :seats_in_use)
-        .and not_change(gitlab_subscription, :seats_owed)
-        .and change(gitlab_subscription_2, :max_seats_used).from(0).to(14)
-        .and change(gitlab_subscription_2, :seats_in_use).from(0).to(13)
-        .and change(gitlab_subscription_2, :seats_owed).from(0).to(12)
+    shared_examples 'updates nothing' do
+      it 'does not update seat columns' do
+        expect do
+          perform_and_reload
+        end.to not_change(gitlab_subscription, :max_seats_used)
+          .and not_change(gitlab_subscription, :seats_in_use)
+          .and not_change(gitlab_subscription, :seats_owed)
+          .and not_change(gitlab_subscription_2, :max_seats_used)
+          .and not_change(gitlab_subscription_2, :seats_in_use)
+          .and not_change(gitlab_subscription_2, :seats_owed)
+      end
     end
-  end
 
-  context 'where the DB is read-only' do
-    let(:db_is_read_only) { true }
+    shared_examples 'updates only paid plans' do
+      it "persists seat attributes after refresh_seat_attributes! for only paid plans" do
+        expect do
+          perform_and_reload
+        end.to not_change(gitlab_subscription, :max_seats_used)
+          .and not_change(gitlab_subscription, :seats_in_use)
+          .and not_change(gitlab_subscription, :seats_owed)
+          .and change(gitlab_subscription_2, :max_seats_used).from(0).to(14)
+          .and change(gitlab_subscription_2, :seats_in_use).from(0).to(13)
+          .and change(gitlab_subscription_2, :seats_owed).from(0).to(12)
+      end
+    end
 
-    include_examples 'updates nothing'
-  end
+    context 'where the DB is read-only' do
+      let(:db_is_read_only) { true }
 
-  context 'when the DB is not read-only' do
-    before do
-      gitlab_subscription.update!(subscription_attrs) if subscription_attrs
+      include_examples 'updates nothing'
     end
 
-    context 'with a free plan' do
-      let(:subscription_attrs) { { hosted_plan: nil } }
+    context 'when the DB is not read-only' do
+      before do
+        gitlab_subscription.update!(subscription_attrs) if subscription_attrs
+
+        allow_next_found_instance_of(GitlabSubscription) do |subscription|
+          allow(subscription).to receive(:refresh_seat_attributes!) do
+            subscription.max_seats_used = subscription.seats + 3
+            subscription.seats_in_use = subscription.seats + 2
+            subscription.seats_owed = subscription.seats + 1
+          end
+        end
+      end
 
-      include_examples 'updates only paid plans'
-    end
+      context 'with a free plan' do
+        let(:subscription_attrs) { { hosted_plan: nil } }
 
-    context 'with a trial plan' do
-      let(:subscription_attrs) { { hosted_plan: bronze_plan, trial: true } }
+        include_examples 'updates only paid plans'
+      end
 
-      include_examples 'updates only paid plans'
+      context 'with a trial plan' do
+        let(:subscription_attrs) { { hosted_plan: bronze_plan, trial: true } }
+
+        include_examples 'updates only paid plans'
+      end
+
+      context 'with a paid plan', :aggregate_failures do
+        before do
+          gitlab_subscription.update!(hosted_plan: bronze_plan)
+          gitlab_subscription_2.update!(hosted_plan: bronze_plan)
+        end
+
+        it 'persists seat attributes after refresh_seat_attributes' do
+          expect do
+            perform_and_reload
+          end.to change(gitlab_subscription, :max_seats_used).from(0).to(4)
+            .and change(gitlab_subscription, :seats_in_use).from(0).to(3)
+            .and change(gitlab_subscription, :seats_owed).from(0).to(2)
+            .and change(gitlab_subscription_2, :max_seats_used).from(0).to(14)
+            .and change(gitlab_subscription_2, :seats_in_use).from(0).to(13)
+            .and change(gitlab_subscription_2, :seats_owed).from(0).to(12)
+        end
+      end
     end
 
-    context 'with a paid plan', :aggregate_failures do
+    context 'when a statement timeout exception is thrown for a subscription' do
       before do
-        gitlab_subscription.update!(hosted_plan: bronze_plan)
-        gitlab_subscription_2.update!(hosted_plan: bronze_plan)
+        allow_next_found_instance_of(GitlabSubscription) do |subscription|
+          allow(subscription).to receive(:refresh_seat_attributes!) do
+            if subscription.id == gitlab_subscription.id
+              raise ActiveRecord::QueryCanceled, 'statement timeout'
+            else
+              subscription.max_seats_used = subscription.seats + 3
+              subscription.seats_in_use = subscription.seats + 2
+              subscription.seats_owed = subscription.seats + 1
+            end
+          end
+        end
+      end
+
+      it 'catches and logs the exception' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
+          an_instance_of(ActiveRecord::QueryCanceled),
+          { gitlab_subscription_id: gitlab_subscription.id,
+            namespace_id: gitlab_subscription.namespace_id })
+
+        perform_and_reload
       end
 
-      it 'persists seat attributes after refresh_seat_attributes' do
+      it 'successfully updates remaining subscriptions' do
         expect do
           perform_and_reload
-        end.to change(gitlab_subscription, :max_seats_used).from(0).to(4)
-          .and change(gitlab_subscription, :seats_in_use).from(0).to(3)
-          .and change(gitlab_subscription, :seats_owed).from(0).to(2)
+        end.to not_change(gitlab_subscription, :max_seats_used).from(0)
+          .and not_change(gitlab_subscription, :seats_in_use).from(0)
+          .and not_change(gitlab_subscription, :seats_owed).from(0)
           .and change(gitlab_subscription_2, :max_seats_used).from(0).to(14)
           .and change(gitlab_subscription_2, :seats_in_use).from(0).to(13)
           .and change(gitlab_subscription_2, :seats_owed).from(0).to(12)