diff options
author | Bob Van Landuyt <bob@gitlab.com> | 2019-07-23 12:39:26 +0000 |
---|---|---|
committer | Bob Van Landuyt <bob@gitlab.com> | 2019-07-23 12:39:26 +0000 |
commit | 825e5c2c1b1cd98f3daf99f22fea910f075f48f9 (patch) | |
tree | a149601d370d94c0bbc9bf7e6639b25470f4a804 | |
parent | d97dac9d6b9a16f1d796e506ceb2d69b87b804c3 (diff) | |
parent | 313f145b5594ebba7ce4675905061144adb3b44a (diff) | |
download | gitlab-ce-825e5c2c1b1cd98f3daf99f22fea910f075f48f9.tar.gz |
Merge branch 'if-64257-active_session_lookup_key_cleanup' into 'master'
Rake task to cleanup expired ActiveSession lookup keys
See merge request gitlab-org/gitlab-ce!30668
-rw-r--r-- | app/models/active_session.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml | 5 | ||||
-rw-r--r-- | doc/raketasks/cleanup.md | 10 | ||||
-rw-r--r-- | lib/tasks/gitlab/cleanup.rake | 52 | ||||
-rw-r--r-- | spec/models/active_session_spec.rb | 2 | ||||
-rw-r--r-- | spec/tasks/gitlab/cleanup_rake_spec.rb | 30 |
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 |