Commit f463a2d9 authored by Imre Farkas's avatar Imre Farkas Committed by Thong Kuah

Do Redis lookup in batches in ActiveSession.sessions_from_ids

By doing smaller mget calls to Redis, it can better schedule the
workload. Currently a single mget with a lot of keys can keep Redis busy
for long, while nothing in its queue gets processed.
parent 4f95a8d7
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
class ActiveSession class ActiveSession
include ActiveModel::Model include ActiveModel::Model
SESSION_BATCH_SIZE = 200
attr_accessor :created_at, :updated_at, attr_accessor :created_at, :updated_at,
:session_id, :ip_address, :session_id, :ip_address,
:browser, :os, :device_name, :device_type, :browser, :os, :device_name, :device_type,
...@@ -106,10 +108,12 @@ class ActiveSession ...@@ -106,10 +108,12 @@ class ActiveSession
Gitlab::Redis::SharedState.with do |redis| Gitlab::Redis::SharedState.with do |redis|
session_keys = session_ids.map { |session_id| "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}" } session_keys = session_ids.map { |session_id| "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}" }
redis.mget(session_keys).compact.map do |raw_session| session_keys.each_slice(SESSION_BATCH_SIZE).flat_map do |session_keys_batch|
# rubocop:disable Security/MarshalLoad redis.mget(session_keys_batch).compact.map do |raw_session|
Marshal.load(raw_session) # rubocop:disable Security/MarshalLoad
# rubocop:enable Security/MarshalLoad Marshal.load(raw_session)
# rubocop:enable Security/MarshalLoad
end
end end
end end
end end
......
---
title: Do Redis lookup in batches in ActiveSession.sessions_from_ids
merge_request: 30561
author:
type: performance
...@@ -132,6 +132,19 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do ...@@ -132,6 +132,19 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
expect(ActiveSession.sessions_from_ids([])).to eq([]) expect(ActiveSession.sessions_from_ids([])).to eq([])
end end
it 'uses redis lookup in batches' do
stub_const('ActiveSession::SESSION_BATCH_SIZE', 1)
redis = double(:redis)
expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis)
sessions = ['session-a', 'session-b']
mget_responses = sessions.map { |session| [Marshal.dump(session)]}
expect(redis).to receive(:mget).twice.and_return(*mget_responses)
expect(ActiveSession.sessions_from_ids([1, 2])).to eql(sessions)
end
end end
describe '.set' do describe '.set' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment