Rake task to cleanup expired ActiveSession lookup keysif-64257-active_session_lookup_key_cleanup

In some cases ActiveSession.cleanup was not called after authentication,
so for some user ActiveSession lookup keys grew without ever cleaning
up. This Rake task manually iterates over the lookup keys and removes
ones without existing ActiveSession.

6 files changed, 102 insertions, 5 deletions

diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index 345767179eb..fdd210f0fba 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -93,12 +93,12 @@ class ActiveSession
   end
 
   def self.list_sessions(user)
-    sessions_from_ids(session_ids_for_user(user))
+    sessions_from_ids(session_ids_for_user(user.id))
   end
 
-  def self.session_ids_for_user(user)
+  def self.session_ids_for_user(user_id)
     Gitlab::Redis::SharedState.with do |redis|
-      redis.smembers(lookup_key_name(user.id))
+      redis.smembers(lookup_key_name(user_id))
     end
   end
 
@@ -129,7 +129,7 @@ class ActiveSession
   end
 
   def self.cleaned_up_lookup_entries(redis, user)
-    session_ids = session_ids_for_user(user)
+    session_ids = session_ids_for_user(user.id)
     entries = raw_active_session_entries(session_ids, user.id)
 
     # remove expired keys.
diff --git a/changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml b/changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml
new file mode 100644
index 00000000000..df3cd98830e
--- /dev/null
+++ b/changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml
@@ -0,0 +1,5 @@
+---
+title: Rake task to cleanup expired ActiveSession lookup keys
+merge_request: 30668
+author:
+type: performance
diff --git a/doc/raketasks/cleanup.md b/doc/raketasks/cleanup.md
index f880f31c39e..832078d23cb 100644
--- a/doc/raketasks/cleanup.md
+++ b/doc/raketasks/cleanup.md
@@ -137,3 +137,13 @@ level with `NICENESS`. Below are the valid levels, but consult
 - `1` or `Realtime`
 - `2` or `Best-effort` (default)
 - `3` or `Idle`
+
+## Remove expired ActiveSession lookup keys
+
+```
+# omnibus-gitlab
+sudo gitlab-rake gitlab:cleanup:sessions:active_sessions_lookup_keys
+
+# installation from source
+bundle exec rake gitlab:cleanup:sessions:active_sessions_lookup_keys RAILS_ENV=production
+```
diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake
index 88172e26c67..4d854cd178d 100644
--- a/lib/tasks/gitlab/cleanup.rake
+++ b/lib/tasks/gitlab/cleanup.rake
@@ -127,6 +127,58 @@ namespace :gitlab do
       end
     end
 
+    namespace :sessions do
+      desc "GitLab | Cleanup | Sessions | Clean ActiveSession lookup keys"
+      task active_sessions_lookup_keys: :gitlab_environment do
+        session_key_pattern = "#{Gitlab::Redis::SharedState::USER_SESSIONS_LOOKUP_NAMESPACE}:*"
+        last_save_check = Time.at(0)
+        wait_time = 10.seconds
+        cursor = 0
+        total_users_scanned = 0
+
+        Gitlab::Redis::SharedState.with do |redis|
+          begin
+            cursor, keys = redis.scan(cursor, match: session_key_pattern)
+            total_users_scanned += keys.count
+
+            if last_save_check < Time.now - 1.second
+              while redis.info('persistence')['rdb_bgsave_in_progress'] == '1'
+                puts "BGSAVE in progress, waiting #{wait_time} seconds"
+                sleep(wait_time)
+              end
+              last_save_check = Time.now
+            end
+
+            keys.each do |key|
+              user_id = key.split(':').last
+
+              lookup_key_count = redis.scard(key)
+
+              session_ids = ActiveSession.session_ids_for_user(user_id)
+              entries = ActiveSession.raw_active_session_entries(session_ids, user_id)
+              session_ids_and_entries = session_ids.zip(entries)
+
+              inactive_session_ids = session_ids_and_entries.map do |session_id, session|
+                session_id if session.nil?
+              end.compact
+
+              redis.pipelined do |conn|
+                inactive_session_ids.each do |session_id|
+                  conn.srem(key, session_id)
+                end
+              end
+
+              if inactive_session_ids
+                puts "deleted #{inactive_session_ids.count} out of #{lookup_key_count} lookup keys for User ##{user_id}"
+              end
+            end
+          end while cursor.to_i != 0
+
+          puts "--- All done! Total number of scanned users: #{total_users_scanned}"
+        end
+      end
+    end
+
     def remove?
       ENV['REMOVE'] == 'true'
     end
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb
index 09c2878663a..2a689754ee0 100644
--- a/spec/models/active_session_spec.rb
+++ b/spec/models/active_session_spec.rb
@@ -114,7 +114,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
         redis.sadd("session:lookup:user:gitlab:#{user.id}", session_ids)
       end
 
-      expect(ActiveSession.session_ids_for_user(user)).to eq(session_ids)
+      expect(ActiveSession.session_ids_for_user(user.id)).to eq(session_ids)
     end
   end
 
diff --git a/spec/tasks/gitlab/cleanup_rake_spec.rb b/spec/tasks/gitlab/cleanup_rake_spec.rb
index 92c094f08a4..4aee6d005a8 100644
--- a/spec/tasks/gitlab/cleanup_rake_spec.rb
+++ b/spec/tasks/gitlab/cleanup_rake_spec.rb
@@ -185,4 +185,34 @@ describe 'gitlab:cleanup rake tasks' do
       end
     end
   end
+
+  context 'sessions' do
+    describe 'gitlab:cleanup:sessions:active_sessions_lookup_keys', :clean_gitlab_redis_shared_state do
+      subject(:rake_task) { run_rake_task('gitlab:cleanup:sessions:active_sessions_lookup_keys') }
+
+      let!(:user) { create(:user) }
+      let(:existing_session_id) { '5' }
+
+      before do
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.set("session:user:gitlab:#{user.id}:#{existing_session_id}",
+                    Marshal.dump(true))
+          redis.sadd("session:lookup:user:gitlab:#{user.id}", (1..10).to_a)
+        end
+      end
+
+      it 'runs the task without errors' do
+        expect { rake_task }.not_to raise_error
+      end
+
+      it 'removes expired active session lookup keys' do
+        Gitlab::Redis::SharedState.with do |redis|
+          lookup_key = "session:lookup:user:gitlab:#{user.id}"
+          expect { subject }.to change { redis.scard(lookup_key) }.from(10).to(1)
+          expect(redis.smembers("session:lookup:user:gitlab:#{user.id}")).to(
+            eql([existing_session_id]))
+        end
+      end
+    end
+  end
 end