summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorImre Farkas <ifarkas@gitlab.com>2019-07-17 11:07:26 +0000
committerThong Kuah <tkuah@gitlab.com>2019-07-17 11:07:26 +0000
commitf463a2d95b657cf9470053eeb3d12d236e3febb4 (patch)
tree08a1a03b62ec7bdf959c946815bb4c63023c63fe
parent4f95a8d7f6612e8df138bd831db7f689a01ef9ca (diff)
downloadgitlab-ce-f463a2d95b657cf9470053eeb3d12d236e3febb4.tar.gz
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.
-rw-r--r--app/models/active_session.rb12
-rw-r--r--changelogs/unreleased/64315-mget_sessions_in_chunks.yml5
-rw-r--r--spec/models/active_session_spec.rb13
3 files changed, 26 insertions, 4 deletions
diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index f355b02c428..345767179eb 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -3,6 +3,8 @@
class ActiveSession
include ActiveModel::Model
+ SESSION_BATCH_SIZE = 200
+
attr_accessor :created_at, :updated_at,
:session_id, :ip_address,
:browser, :os, :device_name, :device_type,
@@ -106,10 +108,12 @@ class ActiveSession
Gitlab::Redis::SharedState.with do |redis|
session_keys = session_ids.map { |session_id| "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}" }
- redis.mget(session_keys).compact.map do |raw_session|
- # rubocop:disable Security/MarshalLoad
- Marshal.load(raw_session)
- # rubocop:enable Security/MarshalLoad
+ session_keys.each_slice(SESSION_BATCH_SIZE).flat_map do |session_keys_batch|
+ redis.mget(session_keys_batch).compact.map do |raw_session|
+ # rubocop:disable Security/MarshalLoad
+ Marshal.load(raw_session)
+ # rubocop:enable Security/MarshalLoad
+ end
end
end
end
diff --git a/changelogs/unreleased/64315-mget_sessions_in_chunks.yml b/changelogs/unreleased/64315-mget_sessions_in_chunks.yml
new file mode 100644
index 00000000000..d50d86726e2
--- /dev/null
+++ b/changelogs/unreleased/64315-mget_sessions_in_chunks.yml
@@ -0,0 +1,5 @@
+---
+title: Do Redis lookup in batches in ActiveSession.sessions_from_ids
+merge_request: 30561
+author:
+type: performance
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb
index 2762eaeccd3..09c2878663a 100644
--- a/spec/models/active_session_spec.rb
+++ b/spec/models/active_session_spec.rb
@@ -132,6 +132,19 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
expect(ActiveSession.sessions_from_ids([])).to eq([])
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
describe '.set' do