summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorImre Farkas <ifarkas@gitlab.com>2019-07-12 14:25:12 +0200
committerImre Farkas <ifarkas@gitlab.com>2019-07-22 17:15:57 +0200
commit313f145b5594ebba7ce4675905061144adb3b44a (patch)
tree33683a02092d00fd587832dd40f99c5452dd2454
parentc5fac1034f43d81a17242d8ade2d7eb8741a72e2 (diff)
downloadgitlab-ce-313f145b5594ebba7ce4675905061144adb3b44a.tar.gz
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.
-rw-r--r--app/models/active_session.rb8
-rw-r--r--changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml5
-rw-r--r--doc/raketasks/cleanup.md10
-rw-r--r--lib/tasks/gitlab/cleanup.rake52
-rw-r--r--spec/models/active_session_spec.rb2
-rw-r--r--spec/tasks/gitlab/cleanup_rake_spec.rb30
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