diff --git a/app/models/user.rb b/app/models/user.rb
index 4a89f414660a3c65fe7a570958b4fcb2d1fae5fc..f76e158b82b62fa701a017955ef5a627785431e3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -30,6 +30,7 @@ class User < ApplicationRecord
include Gitlab::Auth::Otp::Fortinet
include RestrictedSignup
include StripAttribute
+ include EachBatch
DEFAULT_NOTIFICATION_LEVEL = :participating
diff --git a/app/workers/users/deactivate_dormant_users_worker.rb b/app/workers/users/deactivate_dormant_users_worker.rb
index d7ea20e4b62c8b026dd7877fb0b1d0983ac63c2a..b14b7e67450790ab085223f06beec349d65cf834 100644
--- a/app/workers/users/deactivate_dormant_users_worker.rb
+++ b/app/workers/users/deactivate_dormant_users_worker.rb
@@ -10,43 +10,23 @@ class DeactivateDormantUsersWorker # rubocop:disable Scalability/IdempotentWorke
feature_category :utilization
- NUMBER_OF_BATCHES = 50
- BATCH_SIZE = 200
- PAUSE_SECONDS = 0.25
-
def perform
return if Gitlab.com?
return unless ::Gitlab::CurrentSettings.current_application_settings.deactivate_dormant_users
- with_context(caller_id: self.class.name.to_s) do
- NUMBER_OF_BATCHES.times do
- result = User.connection.execute(update_query)
-
- break if result.cmd_tuples == 0
-
- sleep(PAUSE_SECONDS)
- end
- end
+ deactivate_users(User.dormant)
+ deactivate_users(User.with_no_activity)
end
private
- def update_query
- <<~SQL
- UPDATE "users"
- SET "state" = 'deactivated'
- WHERE "users"."id" IN (
- (#{users.dormant.to_sql})
- UNION
- (#{users.with_no_activity.to_sql})
- LIMIT #{BATCH_SIZE}
- )
- SQL
- end
-
- def users
- User.select(:id).limit(BATCH_SIZE)
+ def deactivate_users(scope)
+ with_context(caller_id: self.class.name.to_s) do
+ scope.each_batch do |batch|
+ batch.each(&:deactivate)
+ end
+ end
end
end
end
diff --git a/db/post_migrate/20220802204737_remove_deactivated_user_highest_role_stats.rb b/db/post_migrate/20220802204737_remove_deactivated_user_highest_role_stats.rb
new file mode 100644
index 0000000000000000000000000000000000000000..e23fbfdf7f2a6dd67b138d0e398e135231680e9d
--- /dev/null
+++ b/db/post_migrate/20220802204737_remove_deactivated_user_highest_role_stats.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class RemoveDeactivatedUserHighestRoleStats < Gitlab::Database::Migration[2.0]
+ disable_ddl_transaction!
+
+ restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+ def up
+ # This migration is applicable to self-managed instances that may utilize the
+ # dormant user deactivation feature. This feature is not enabled on Gitlab.com.
+ return if Gitlab.com?
+
+ users_table = define_batchable_model('users')
+ user_highest_roles_table = define_batchable_model('user_highest_roles')
+
+ users_table.where(state: 'deactivated').each_batch do |users_batch|
+ user_ids = users_batch.pluck(:id)
+ user_highest_roles_table.where(user_id: user_ids).delete_all
+ end
+ end
+
+ def down
+ # no-op
+
+ # This migration removes entries from the UserHighestRole table and cannot be reversed
+ end
+end
diff --git a/db/schema_migrations/20220802204737 b/db/schema_migrations/20220802204737
new file mode 100644
index 0000000000000000000000000000000000000000..faf1e6b89baa014f86e4c58c0b93236509f5f02a
--- /dev/null
+++ b/db/schema_migrations/20220802204737
@@ -0,0 +1 @@
+4de7fddbc2f44cf1450af25bd55a5f2586c3daf79b1443ec26ba9d47002707d7
\ No newline at end of file
diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb
index c423340a5725aeb3cb80b6618ebf76280425dd0c..f21f1ac5e5221d87df54d548fe8a45ab16c33a7e 100644
--- a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb
@@ -37,12 +37,6 @@
freeze_time { example.run }
end
- before do
- User.class_eval do
- include EachBatch
- end
- end
-
it 'returns the final expected delay' do
Sidekiq::Testing.fake! do
final_delay = model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2)
diff --git a/spec/migrations/20220802204737_remove_deactivated_user_highest_role_stats_spec.rb b/spec/migrations/20220802204737_remove_deactivated_user_highest_role_stats_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..3ea286ca138dd74da9cfb6b997c7378df728559d
--- /dev/null
+++ b/spec/migrations/20220802204737_remove_deactivated_user_highest_role_stats_spec.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe RemoveDeactivatedUserHighestRoleStats do
+ let!(:users) { table(:users) }
+ let!(:user_highest_roles) { table(:user_highest_roles) }
+
+ let!(:user1) do
+ users.create!(username: 'user1', email: 'user1@example.com', projects_limit: 10, state: 'active')
+ end
+
+ let!(:user2) do
+ users.create!(username: 'user2', email: 'user2@example.com', projects_limit: 10, state: 'deactivated')
+ end
+
+ let!(:highest_role1) { user_highest_roles.create!(user_id: user1.id) }
+ let!(:highest_role2) { user_highest_roles.create!(user_id: user2.id) }
+
+ describe '#up' do
+ context 'when on gitlab.com' do
+ it 'does not change user highest role records' do
+ allow(Gitlab).to receive(:com?).and_return(true)
+ expect { migrate! }.not_to change(user_highest_roles, :count)
+ end
+ end
+
+ context 'when not on gitlab.com' do
+ it 'removes all user highest role records for deactivated users' do
+ allow(Gitlab).to receive(:com?).and_return(false)
+ migrate!
+ expect(user_highest_roles.pluck(:user_id)).to contain_exactly(
+ user1.id
+ )
+ end
+ end
+ end
+end
diff --git a/spec/workers/users/deactivate_dormant_users_worker_spec.rb b/spec/workers/users/deactivate_dormant_users_worker_spec.rb
index 297301c45e2925d65afbedd81b78d2ccb09155c8..263ca31e0a0ee8f7f1c960c760ac9afce5df3b76 100644
--- a/spec/workers/users/deactivate_dormant_users_worker_spec.rb
+++ b/spec/workers/users/deactivate_dormant_users_worker_spec.rb
@@ -25,20 +25,13 @@
context 'when automatic deactivation of dormant users is enabled' do
before do
stub_application_setting(deactivate_dormant_users: true)
- stub_const("#{described_class.name}::PAUSE_SECONDS", 0)
end
it 'deactivates dormant users' do
- freeze_time do
- stub_const("#{described_class.name}::BATCH_SIZE", 1)
-
- expect(worker).to receive(:sleep).twice
-
- worker.perform
+ worker.perform
- expect(User.dormant.count).to eq(0)
- expect(User.with_no_activity.count).to eq(0)
- end
+ expect(User.dormant.count).to eq(0)
+ expect(User.with_no_activity.count).to eq(0)
end
where(:user_type, :expected_state) do
@@ -78,6 +71,14 @@
expect(inactive_recently_created.reload.state).to eq('active')
end
+
+ it 'triggers update of highest user role for deactivated users', :clean_gitlab_redis_shared_state do
+ [dormant, inactive].each do |user|
+ expect(UpdateHighestRoleWorker).to receive(:perform_in).with(anything, user.id)
+ end
+
+ worker.perform
+ end
end
context 'when automatic deactivation of dormant users is disabled' do