Merge branch '1334-use-redis-sessions-store' into 'master'

Use Redis session instance

See merge request gitlab-org/gitlab!74202

app/models/active_session.rb

@@ -21,6 +21,7 @@
 #
 class ActiveSession
   include ActiveModel::Model
+  include ::Gitlab::Redis::SessionsStoreHelper
 
   SESSION_BATCH_SIZE = 200
   ALLOWED_NUMBER_OF_ACTIVE_SESSIONS = 100
@@ -43,7 +44,7 @@ class ActiveSession
   end
 
   def self.set(user, request)
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       session_private_id = request.session.id.private_id
       client = DeviceDetector.new(request.user_agent)
       timestamp = Time.current
@@ -76,7 +77,7 @@ class ActiveSession
   end
 
   def self.list(user)
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       cleaned_up_lookup_entries(redis, user).map do |raw_session|
         load_raw_session(raw_session)
       end
@@ -84,7 +85,7 @@ class ActiveSession
   end
 
   def self.cleanup(user)
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       clean_up_old_sessions(redis, user)
       cleaned_up_lookup_entries(redis, user)
     end
@@ -104,7 +105,7 @@ class ActiveSession
   def self.destroy_session(user, session_id)
     return unless session_id
 
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       destroy_sessions(redis, user, [session_id].compact)
     end
   end
@@ -113,7 +114,7 @@ class ActiveSession
     sessions = not_impersonated(user)
     sessions.reject! { |session| session.current?(current_rack_session) } if current_rack_session
 
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       session_ids = (sessions.map(&:session_id) | sessions.map(&:session_private_id)).compact
       destroy_sessions(redis, user, session_ids) if session_ids.any?
     end
@@ -124,15 +125,15 @@ class ActiveSession
   end
 
   def self.rack_key_name(session_id)
-    "#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}"
+    "#{Gitlab::Redis::Sessions::SESSION_NAMESPACE}:#{session_id}"
   end
 
   def self.key_name(user_id, session_id = '*')
-    "#{Gitlab::Redis::SharedState::USER_SESSIONS_NAMESPACE}:#{user_id}:#{session_id}"
+    "#{Gitlab::Redis::Sessions::USER_SESSIONS_NAMESPACE}:#{user_id}:#{session_id}"
   end
 
   def self.lookup_key_name(user_id)
-    "#{Gitlab::Redis::SharedState::USER_SESSIONS_LOOKUP_NAMESPACE}:#{user_id}"
+    "#{Gitlab::Redis::Sessions::USER_SESSIONS_LOOKUP_NAMESPACE}:#{user_id}"
   end
 
   def self.list_sessions(user)
@@ -143,7 +144,7 @@ class ActiveSession
   #
   # Returns an array of strings
   def self.session_ids_for_user(user_id)
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       redis.smembers(lookup_key_name(user_id))
     end
   end
@@ -156,7 +157,7 @@ class ActiveSession
   def self.sessions_from_ids(session_ids)
     return [] if session_ids.empty?
 
-    Gitlab::Redis::SharedState.with do |redis|
+    redis_store_class.with do |redis|
       session_keys = rack_session_keys(session_ids)
 
       session_keys.each_slice(SESSION_BATCH_SIZE).flat_map do |session_keys_batch|
@@ -228,9 +229,9 @@ class ActiveSession
     # only the single key entries are automatically expired by redis, the
     # lookup entries in the set need to be removed manually.
     session_ids_and_entries = session_ids.zip(entries)
-    redis.pipelined do
+    redis.pipelined do |pipeline|
       session_ids_and_entries.reject { |_session_id, entry| entry }.each do |session_id, _entry|
-        redis.srem(lookup_key_name(user.id), session_id)
+        pipeline.srem(lookup_key_name(user.id), session_id)
       end
     end
 

config/feature_flags/development/use_multi_store.yml

 ---
 name: use_multi_store
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73660
-rollout_issue_url:
+rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1429
 milestone: '14.5'
 type: development
 group: group::memory

config/initializers/session_store.rb

@@ -19,31 +19,22 @@ cookie_key = if Rails.env.development?
                "_gitlab_session"
              end
 
-if Gitlab::Utils.to_boolean(ENV['GITLAB_REDIS_STORE_WITH_SESSION_STORE'], default: true)
-  store = Gitlab::Redis::SharedState.store(
-    namespace: Gitlab::Redis::SharedState::SESSION_NAMESPACE
+store = if Gitlab::Utils.to_boolean(ENV['GITLAB_USE_REDIS_SESSIONS_STORE'], default: true)
+          Gitlab::Redis::Sessions.store(
+            namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE
           )
-
-  Gitlab::Application.config.session_store(
-    :redis_store, # Using the cookie_store would enable session replay attacks.
-    redis_store: store,
-    key: cookie_key,
-    secure: Gitlab.config.gitlab.https,
-    httponly: true,
-    expires_in: Settings.gitlab['session_expire_delay'] * 60,
-    path: Rails.application.config.relative_url_root.presence || '/'
+        else
+          Gitlab::Redis::SharedState.store(
+            namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE
           )
-else
-  sessions_config = Gitlab::Redis::SharedState.params
-  sessions_config[:namespace] = Gitlab::Redis::SharedState::SESSION_NAMESPACE
+        end
 
-  Gitlab::Application.config.session_store(
+Gitlab::Application.config.session_store(
   :redis_store, # Using the cookie_store would enable session replay attacks.
-    servers: sessions_config,
+  redis_store: store,
   key: cookie_key,
   secure: Gitlab.config.gitlab.https,
   httponly: true,
   expires_in: Settings.gitlab['session_expire_delay'] * 60,
   path: Rails.application.config.relative_url_root.presence || '/'
-  )
-end
+)

doc/development/session.md

@@ -51,12 +51,12 @@ Session data can be accessed directly through Redis. This can let you check up o
 
 ```ruby
 # Get a list of sessions
-session_ids = Gitlab::Redis::SharedState.with do |redis|
-  redis.smembers("#{Gitlab::Redis::SharedState::USER_SESSIONS_LOOKUP_NAMESPACE}:#{user.id}")
+session_ids = Gitlab::Redis::Sessions.with do |redis|
+  redis.smembers("#{Gitlab::Redis::Sessions::USER_SESSIONS_LOOKUP_NAMESPACE}:#{user.id}")
 end
 
 # Retrieve a specific session
-session_data = Gitlab::Redis::SharedState.with { |redis| redis.get("#{Gitlab::Redis::SharedState::SESSION_NAMESPACE}:#{session_id}") }
+session_data = Gitlab::Redis::Sessions.with { |redis| redis.get("#{Gitlab::Redis::Sessions::SESSION_NAMESPACE}:#{session_id}") }
 Marshal.load(session_data)
 ```
 

ee/lib/gitlab/auth/otp/session_enforcer.rb

@@ -4,20 +4,20 @@ module Gitlab
   module Auth
     module Otp
       class SessionEnforcer
-        OTP_SESSIONS_NAMESPACE = 'session:otp'
+        include ::Gitlab::Redis::SessionsStoreHelper
 
         def initialize(key)
           @key = key
         end
 
         def update_session
-          Gitlab::Redis::SharedState.with do |redis|
+          redis_store_class.with do |redis|
             redis.setex(key_name, session_expiry_in_seconds, true)
           end
         end
 
         def access_restricted?
-          Gitlab::Redis::SharedState.with do |redis|
+          redis_store_class.with do |redis|
             !redis.get(key_name)
           end
         end
@@ -27,7 +27,7 @@ module Gitlab
         attr_reader :key
 
         def key_name
-          @key_name ||= "#{OTP_SESSIONS_NAMESPACE}:#{key.id}"
+          @key_name ||= "#{Gitlab::Redis::Sessions::OTP_SESSIONS_NAMESPACE}:#{key.id}"
         end
 
         def session_expiry_in_seconds

ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
           expect(response).to have_gitlab_http_status(:not_found)
         end
 
-        context 'with an active session', :clean_gitlab_redis_shared_state do
+        context 'with an active session', :clean_gitlab_redis_sessions do
           let(:session_id) { '42' }
           let(:session_time) { 5.minutes.ago }
           let(:stored_session) do
@@ -53,7 +53,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
           end
 
           before do
-            Gitlab::Redis::SharedState.with do |redis|
+            Gitlab::Redis::Sessions.with do |redis|
               redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
               redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
             end

ee/spec/features/users/login_spec.rb

@@ -85,7 +85,7 @@ RSpec.describe 'Login' do
           expect(page.body).to have_link('Register now', href: new_user_registration_path)
         end
 
-        describe 'with two-factor authentication required', :clean_gitlab_redis_shared_state do
+        describe 'with two-factor authentication required', :clean_gitlab_redis_sessions do
           let_it_be(:user) { create(:user) }
           let_it_be(:smartcard_identity) { create(:smartcard_identity, user: user) }
 

ee/spec/lib/gitlab/auth/group_saml/session_enforcer_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
         allow(saml_provider).to receive(:git_check_enforced?).and_return(true)
       end
 
-      context 'with an active session', :clean_gitlab_redis_shared_state do
+      context 'with an active session', :clean_gitlab_redis_sessions do
         let(:session_id) { '42' }
         let(:session_time) { 5.minutes.ago }
         let(:stored_session) do
@@ -35,7 +35,7 @@ RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
         end
 
         before do
-          Gitlab::Redis::SharedState.with do |redis|
+          Gitlab::Redis::Sessions.with do |redis|
             redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
             redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
           end
@@ -63,14 +63,14 @@ RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
           end
         end
 
-        context 'with two active sessions', :clean_gitlab_redis_shared_state do
+        context 'with two active sessions', :clean_gitlab_redis_sessions do
           let(:second_session_id) { '52' }
           let(:second_stored_session) do
             { 'active_group_sso_sign_ins' => { create(:saml_provider, enforced_sso: true).id => session_time } }
           end
 
           before do
-            Gitlab::Redis::SharedState.with do |redis|
+            Gitlab::Redis::Sessions.with do |redis|
               redis.set("session:gitlab:#{second_session_id}", Marshal.dump(second_stored_session))
               redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id, second_session_id])
             end
@@ -79,7 +79,7 @@ RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
           it_behaves_like 'not enforced'
         end
 
-        context 'with two active sessions for the same provider and one pre-sso', :clean_gitlab_redis_shared_state do
+        context 'with two active sessions for the same provider and one pre-sso', :clean_gitlab_redis_sessions do
           let(:second_session_id) { '52' }
           let(:third_session_id) { '62' }
           let(:second_stored_session) do
@@ -87,7 +87,7 @@ RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
           end
 
           before do
-            Gitlab::Redis::SharedState.with do |redis|
+            Gitlab::Redis::Sessions.with do |redis|
               redis.set("session:gitlab:#{second_session_id}", Marshal.dump(second_stored_session))
               redis.set("session:gitlab:#{third_session_id}", Marshal.dump({}))
               redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id, second_session_id, third_session_id])
@@ -192,14 +192,14 @@ RSpec.describe Gitlab::Auth::GroupSaml::SessionEnforcer do
         allow(saml_provider).to receive(:git_check_enforced?).and_return(false)
       end
 
-      context 'with an active session', :clean_gitlab_redis_shared_state do
+      context 'with an active session', :clean_gitlab_redis_sessions do
         let(:session_id) { '42' }
         let(:stored_session) do
           { 'active_group_sso_sign_ins' => { saml_provider.id => 5.minutes.ago } }
         end
 
         before do
-          Gitlab::Redis::SharedState.with do |redis|
+          Gitlab::Redis::Sessions.with do |redis|
             redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
             redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
           end

ee/spec/lib/gitlab/auth/otp/session_enforcer_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Auth::Otp::SessionEnforcer, :clean_gitlab_redis_shared_state do
+RSpec.describe Gitlab::Auth::Otp::SessionEnforcer, :clean_gitlab_redis_sessions do
   let_it_be(:key) { create(:key)}
 
   describe '#update_session' do
@@ -13,12 +13,12 @@ RSpec.describe Gitlab::Auth::Otp::SessionEnforcer, :clean_gitlab_redis_shared_st
     end
 
     it 'registers a session in Redis' do
-      expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis)
+      expect(Gitlab::Redis::Sessions).to receive(:with).and_yield(redis)
       session_expiry_in_seconds = Gitlab::CurrentSettings.git_two_factor_session_expiry.minutes.to_i
 
       expect(redis).to(
         receive(:setex)
-          .with("#{described_class::OTP_SESSIONS_NAMESPACE}:#{key.id}",
+          .with("#{::Gitlab::Redis::Sessions::OTP_SESSIONS_NAMESPACE}:#{key.id}",
                 session_expiry_in_seconds,
                 true)
           .once)
@@ -48,8 +48,8 @@ RSpec.describe Gitlab::Auth::Otp::SessionEnforcer, :clean_gitlab_redis_shared_st
 
     context 'with existing session' do
       before do
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.set("#{described_class::OTP_SESSIONS_NAMESPACE}:#{key.id}", true )
+        Gitlab::Redis::Sessions.with do |redis|
+          redis.set("#{::Gitlab::Redis::Sessions::OTP_SESSIONS_NAMESPACE}:#{key.id}", true )
         end
       end
 

ee/spec/lib/gitlab/auth/smartcard/session_enforcer_spec.rb

@@ -29,14 +29,14 @@ RSpec.describe Gitlab::Auth::Smartcard::SessionEnforcer do
       stub_smartcard_setting(enabled: true, required_for_git_access: true)
     end
 
-    context 'with a smartcard session', :clean_gitlab_redis_shared_state do
+    context 'with a smartcard session', :clean_gitlab_redis_sessions do
       let(:session_id) { '42' }
       let(:stored_session) do
         { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
       end
 
       before do
-        Gitlab::Redis::SharedState.with do |redis|
+        Gitlab::Redis::Sessions.with do |redis|
           redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
           redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
         end

ee/spec/lib/gitlab/auth/smartcard/session_spec.rb

@@ -8,14 +8,14 @@ RSpec.describe Gitlab::Auth::Smartcard::Session do
 
     subject { described_class.new.active?(user) }
 
-    context 'with a smartcard session', :clean_gitlab_redis_shared_state do
+    context 'with a smartcard session', :clean_gitlab_redis_sessions do
       let(:session_id) { '42' }
       let(:stored_session) do
         { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
       end
 
       before do
-        Gitlab::Redis::SharedState.with do |redis|
+        Gitlab::Redis::Sessions.with do |redis|
           redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
           redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
         end

ee/spec/lib/gitlab/git_access_spec.rb

@@ -712,14 +712,14 @@ RSpec.describe Gitlab::GitAccess do
       project.add_developer(user)
     end
 
-    context 'user with a smartcard session', :clean_gitlab_redis_shared_state do
+    context 'user with a smartcard session', :clean_gitlab_redis_sessions do
       let(:session_id) { '42' }
       let(:stored_session) do
         { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
       end
 
       before do
-        Gitlab::Redis::SharedState.with do |redis|
+        Gitlab::Redis::Sessions.with do |redis|
           redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
           redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
         end
@@ -772,10 +772,10 @@ RSpec.describe Gitlab::GitAccess do
       stub_licensed_features(git_two_factor_enforcement: true)
     end
 
-    context 'with an OTP session', :clean_gitlab_redis_shared_state do
+    context 'with an OTP session', :clean_gitlab_redis_sessions do
       before do
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.set("#{Gitlab::Auth::Otp::SessionEnforcer::OTP_SESSIONS_NAMESPACE}:#{key.id}", true)
+        Gitlab::Redis::Sessions.with do |redis|
+          redis.set("#{Gitlab::Redis::Sessions::OTP_SESSIONS_NAMESPACE}:#{key.id}", true)
         end
       end
 
@@ -803,11 +803,11 @@ RSpec.describe Gitlab::GitAccess do
 
         def stub_redis
           redis = double(:redis)
-          expect(Gitlab::Redis::SharedState).to receive(:with).at_most(:twice).and_yield(redis)
+          expect(Gitlab::Redis::Sessions).to receive(:with).at_most(:twice).and_yield(redis)
 
           expect(redis).to(
             receive(:get)
-              .with("#{Gitlab::Auth::Otp::SessionEnforcer::OTP_SESSIONS_NAMESPACE}:#{key.id}"))
+              .with("#{Gitlab::Redis::Sessions::OTP_SESSIONS_NAMESPACE}:#{key.id}"))
                        .at_most(:twice)
                        .and_return(value_of_key)
         end

ee/spec/requests/api/internal/base_spec.rb

@@ -154,14 +154,14 @@ RSpec.describe API::Internal::Base do
         project.add_developer(user)
       end
 
-      context 'user with a smartcard session', :clean_gitlab_redis_shared_state do
+      context 'user with a smartcard session', :clean_gitlab_redis_sessions do
         let(:session_id) { '42' }
         let(:stored_session) do
           { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
         end
 
         before do
-          Gitlab::Redis::SharedState.with do |redis|
+          Gitlab::Redis::Sessions.with do |redis|
             redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
             redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
           end

ee/spec/requests/repositories/git_http_controller_spec.rb

@@ -54,14 +54,14 @@ RSpec.describe Repositories::GitHttpController, type: :request do
         project.add_developer(user)
       end
 
-      context 'user with a smartcard session', :clean_gitlab_redis_shared_state do
+      context 'user with a smartcard session', :clean_gitlab_redis_sessions do
         let(:session_id) { '42' }
         let(:stored_session) do
           { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
         end
 
         before do
-          Gitlab::Redis::SharedState.with do |redis|
+          Gitlab::Redis::Sessions.with do |redis|
             redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
             redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
           end

lib/gitlab/anonymous_session.rb

@@ -2,12 +2,14 @@
 
 module Gitlab
   class AnonymousSession
+    include ::Gitlab::Redis::SessionsStoreHelper
+
     def initialize(remote_ip)
       @remote_ip = remote_ip
     end
 
     def count_session_ip
-      Gitlab::Redis::SharedState.with do |redis|
+      redis_store_class.with do |redis|
         redis.pipelined do
           redis.incr(session_lookup_name)
           redis.expire(session_lookup_name, 24.hours)
@@ -16,13 +18,13 @@ module Gitlab
     end
 
     def session_count
-      Gitlab::Redis::SharedState.with do |redis|
+      redis_store_class.with do |redis|
         redis.get(session_lookup_name).to_i
       end
     end
 
     def cleanup_session_per_ip_count
-      Gitlab::Redis::SharedState.with do |redis|
+      redis_store_class.with do |redis|
         redis.del(session_lookup_name)
       end
     end
@@ -32,7 +34,7 @@ module Gitlab
     attr_reader :remote_ip
 
     def session_lookup_name
-      @session_lookup_name ||= "#{Gitlab::Redis::SharedState::IP_SESSIONS_LOOKUP_NAMESPACE}:#{remote_ip}"
+      @session_lookup_name ||= "#{Gitlab::Redis::Sessions::IP_SESSIONS_LOOKUP_NAMESPACE}:#{remote_ip}"
     end
   end
 end

lib/gitlab/redis/multi_store.rb

@@ -194,7 +194,7 @@ module Gitlab
 
       def increment_method_missing_count(command_name)
         @method_missing_counter ||= Gitlab::Metrics.counter(:gitlab_redis_multi_store_method_missing_total, 'Client side Redis MultiStore method missing')
-        @method_missing_counter.increment(command: command_name, innamece_name: instance_name)
+        @method_missing_counter.increment(command: command_name, instance_name: instance_name)
       end
 
       def validate_stores!

lib/gitlab/redis/sessions.rb

@@ -3,10 +3,46 @@
 module Gitlab
   module Redis
     class Sessions < ::Gitlab::Redis::Wrapper
+      SESSION_NAMESPACE = 'session:gitlab'
+      USER_SESSIONS_NAMESPACE = 'session:user:gitlab'
+      USER_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:user:gitlab'
+      IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab2'
+      OTP_SESSIONS_NAMESPACE = 'session:otp'
+
+      class << self
         # The data we store on Sessions used to be stored on SharedState.
-      def self.config_fallback
+        def config_fallback
           SharedState
         end
+
+        private
+
+        def redis
+          # Don't use multistore if redis.sessions configuration is not provided
+          return super if config_fallback?
+
+          primary_store = ::Redis.new(params)
+          secondary_store = ::Redis.new(config_fallback.params)
+
+          MultiStore.new(primary_store, secondary_store, name)
+        end
+      end
+
+      def store(extras = {})
+        # Don't use multistore if redis.sessions configuration is not provided
+        return super if self.class.config_fallback?
+
+        primary_store = create_redis_store(redis_store_options, extras)
+        secondary_store = create_redis_store(self.class.config_fallback.params, extras)
+
+        MultiStore.new(primary_store, secondary_store, self.class.name)
+      end
+
+      private
+
+      def create_redis_store(options, extras)
+        ::Redis::Store.new(options.merge(extras))
+      end
     end
   end
 end

lib/gitlab/redis/sessions_store_helper.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Redis
+    module SessionsStoreHelper
+      extend ActiveSupport::Concern
+
+      module StoreMethods
+        def redis_store_class
+          use_redis_session_store? ? Gitlab::Redis::Sessions : Gitlab::Redis::SharedState
+        end
+
+        private
+
+        def use_redis_session_store?
+          Gitlab::Utils.to_boolean(ENV['GITLAB_USE_REDIS_SESSIONS_STORE'], default: true)
+        end
+      end
+
+      include StoreMethods
+
+      included do
+        extend StoreMethods
+      end
+    end
+  end
+end

lib/gitlab/redis/shared_state.rb

@@ -3,10 +3,6 @@
 module Gitlab
   module Redis
     class SharedState < ::Gitlab::Redis::Wrapper
-      SESSION_NAMESPACE = 'session:gitlab'
-      USER_SESSIONS_NAMESPACE = 'session:user:gitlab'
-      USER_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:user:gitlab'
-      IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab2'
     end
   end
 end

lib/gitlab/redis/wrapper.rb

@@ -28,7 +28,7 @@ module Gitlab
         end
 
         def pool
-          @pool ||= ConnectionPool.new(size: pool_size) { ::Redis.new(params) }
+          @pool ||= ConnectionPool.new(size: pool_size) { redis }
         end
 
         def pool_size
@@ -67,6 +67,10 @@ module Gitlab
           File.expand_path('../../..', __dir__)
         end
 
+        def config_fallback?
+          config_file_name == config_fallback&.config_file_name
+        end
+
         def config_file_name
           [
             # Instance specific config sources:
@@ -100,6 +104,12 @@ module Gitlab
 
           "::Gitlab::Instrumentation::Redis::#{store_name}".constantize
         end
+
+        private
+
+        def redis
+          ::Redis.new(params)
+        end
       end
 
       def initialize(rails_env = nil)

lib/tasks/gitlab/cleanup.rake

@@ -100,13 +100,15 @@ namespace :gitlab do
     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}:*"
+        use_redis_session_store = Gitlab::Utils.to_boolean(ENV['GITLAB_USE_REDIS_SESSIONS_STORE'], default: true)
+        redis_store_class = use_redis_session_store ? Gitlab::Redis::Sessions : Gitlab::Redis::SharedState
+        session_key_pattern = "#{Gitlab::Redis::Sessions::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|
+        redis_store_class.with do |redis|
           begin
             cursor, keys = redis.scan(cursor, match: session_key_pattern)
             total_users_scanned += keys.count

spec/channels/application_cable/connection_spec.rb

@@ -2,12 +2,12 @@
 
 require 'spec_helper'
 
-RSpec.describe ApplicationCable::Connection, :clean_gitlab_redis_shared_state do
+RSpec.describe ApplicationCable::Connection, :clean_gitlab_redis_sessions do
   let(:session_id) { Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d') }
 
   context 'when session cookie is set' do
     before do
-      Gitlab::Redis::SharedState.with do |redis|
+      Gitlab::Redis::Sessions.with do |redis|
         redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
       end
 

spec/features/users/active_sessions_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Active user sessions', :clean_gitlab_redis_shared_state do
+RSpec.describe 'Active user sessions', :clean_gitlab_redis_sessions do
   it 'successful login adds a new active user login' do
     now = Time.zone.parse('2018-03-12 09:06')
     Timecop.freeze(now) do
@@ -29,13 +29,13 @@ RSpec.describe 'Active user sessions', :clean_gitlab_redis_shared_state do
   it 'successful login cleans up obsolete entries' do
     user = create(:user)
 
-    Gitlab::Redis::SharedState.with do |redis|
+    Gitlab::Redis::Sessions.with do |redis|
       redis.sadd("session:lookup:user:gitlab:#{user.id}", '59822c7d9fcdfa03725eff41782ad97d')
     end
 
     gitlab_sign_in(user)
 
-    Gitlab::Redis::SharedState.with do |redis|
+    Gitlab::Redis::Sessions.with do |redis|
       expect(redis.smembers("session:lookup:user:gitlab:#{user.id}")).not_to include '59822c7d9fcdfa03725eff41782ad97d'
     end
   end
@@ -44,14 +44,14 @@ RSpec.describe 'Active user sessions', :clean_gitlab_redis_shared_state do
     user = create(:user)
     personal_access_token = create(:personal_access_token, user: user)
 
-    Gitlab::Redis::SharedState.with do |redis|
+    Gitlab::Redis::Sessions.with do |redis|
       redis.sadd("session:lookup:user:gitlab:#{user.id}", '59822c7d9fcdfa03725eff41782ad97d')
     end
 
     visit user_path(user, :atom, private_token: personal_access_token.token)
     expect(page.status_code).to eq 200
 
-    Gitlab::Redis::SharedState.with do |redis|
+    Gitlab::Redis::Sessions.with do |redis|
       expect(redis.smembers("session:lookup:user:gitlab:#{user.id}")).to include '59822c7d9fcdfa03725eff41782ad97d'
     end
   end

spec/features/users/login_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
+RSpec.describe 'Login', :clean_gitlab_redis_sessions do
   include TermsHelper
   include UserLoginHelper
   include SessionHelpers
@@ -84,7 +84,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
       expect(page).to have_content('Your account has been blocked.')
     end
 
-    it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do
+    it 'does not update Devise trackable attributes' do
       expect(authentication_metrics)
         .to increment(:user_blocked_counter)
         .and increment(:user_unauthenticated_counter)
@@ -161,7 +161,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
       expect(page).to have_content('Invalid login or password.')
     end
 
-    it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do
+    it 'does not update Devise trackable attributes' do
       expect(authentication_metrics)
         .to increment(:user_unauthenticated_counter)
         .and increment(:user_password_invalid_counter)

spec/initializers/session_store_spec.rb

@@ -10,25 +10,37 @@ RSpec.describe 'Session initializer for GitLab' do
   end
 
   describe 'config#session_store' do
-    context 'when the GITLAB_REDIS_STORE_WITH_SESSION_STORE env is not set' do
+    context 'when the GITLAB_USE_REDIS_SESSIONS_STORE env is not set' do
       before do
-        stub_env('GITLAB_REDIS_STORE_WITH_SESSION_STORE', nil)
+        stub_env('GITLAB_USE_REDIS_SESSIONS_STORE', nil)
       end
 
-      it 'initialized as a redis_store with a proper Redis::Store instance' do
+      it 'initialized with Multistore as ENV var defaults to true' do
         expect(subject).to receive(:session_store).with(:redis_store, a_hash_including(redis_store: kind_of(::Redis::Store)))
 
         load_session_store
       end
     end
 
-    context 'when the GITLAB_REDIS_STORE_WITH_SESSION_STORE env is disabled' do
+    context 'when the GITLAB_USE_REDIS_SESSIONS_STORE env is disabled' do
       before do
-        stub_env('GITLAB_REDIS_STORE_WITH_SESSION_STORE', false)
+        stub_env('GITLAB_USE_REDIS_SESSIONS_STORE', false)
       end
 
       it 'initialized as a redis_store with a proper servers configuration' do
-        expect(subject).to receive(:session_store).with(:redis_store, a_hash_including(servers: kind_of(Hash)))
+        expect(subject).to receive(:session_store).with(:redis_store, a_hash_including(redis_store: kind_of(Redis::Store)))
+
+        load_session_store
+      end
+    end
+
+    context 'when the GITLAB_USE_REDIS_SESSIONS_STORE env is enabled' do
+      before do
+        stub_env('GITLAB_USE_REDIS_SESSIONS_STORE', true)
+      end
+
+      it 'initialized as a redis_store with a proper servers configuration' do
+        expect(subject).to receive(:session_store).with(:redis_store, a_hash_including(redis_store: kind_of(::Redis::Store)))
 
         load_session_store
       end

spec/lib/gitlab/anonymous_session_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
+RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_sessions do
   let(:default_session_id) { '6919a6f1bb119dd7396fadc38fd18d0d' }
   let(:additional_session_id) { '7919a6f1bb119dd7396fadc38fd18d0d' }
 
@@ -16,7 +16,7 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
     it 'adds session id to proper key' do
       subject.count_session_ip
 
-      Gitlab::Redis::SharedState.with do |redis|
+      Gitlab::Redis::Sessions.with do |redis|
         expect(redis.get("session:lookup:ip:gitlab2:127.0.0.1").to_i).to eq 1
       end
     end
@@ -25,7 +25,7 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
       freeze_time do
         subject.count_session_ip
 
-        Gitlab::Redis::SharedState.with do |redis|
+        Gitlab::Redis::Sessions.with do |redis|
           expect(redis.ttl("session:lookup:ip:gitlab2:127.0.0.1")).to eq(24.hours.to_i)
         end
       end
@@ -36,7 +36,7 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
         subject.count_session_ip
         new_anonymous_session.count_session_ip
 
-        Gitlab::Redis::SharedState.with do |redis|
+        Gitlab::Redis::Sessions.with do |redis|
           expect(redis.get("session:lookup:ip:gitlab2:127.0.0.1").to_i).to eq(2)
         end
       end
@@ -45,7 +45,7 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
 
   describe '#stored_sessions' do
     it 'returns all anonymous sessions per ip' do
-      Gitlab::Redis::SharedState.with do |redis|
+      Gitlab::Redis::Sessions.with do |redis|
         redis.set("session:lookup:ip:gitlab2:127.0.0.1", 2)
       end
 
@@ -54,13 +54,13 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
   end
 
   it 'removes obsolete lookup through ip entries' do
-    Gitlab::Redis::SharedState.with do |redis|
+    Gitlab::Redis::Sessions.with do |redis|
       redis.set("session:lookup:ip:gitlab2:127.0.0.1", 2)
     end
 
     subject.cleanup_session_per_ip_count
 
-    Gitlab::Redis::SharedState.with do |redis|
+    Gitlab::Redis::Sessions.with do |redis|
       expect(redis.exists("session:lookup:ip:gitlab2:127.0.0.1")).to eq(false)
     end
   end

spec/lib/gitlab/redis/multi_store_spec.rb

@@ -114,6 +114,12 @@ RSpec.describe Gitlab::Redis::MultiStore do
     end
 
     RSpec.shared_examples_for 'fallback read from the secondary store' do
+      let(:counter) { Gitlab::Metrics::NullMetric.instance }
+
+      before do
+        allow(Gitlab::Metrics).to receive(:counter).and_return(counter)
+      end
+
       it 'fallback and execute on secondary instance' do
         expect(secondary_store).to receive(name).with(*args).and_call_original
 
@@ -128,7 +134,7 @@ RSpec.describe Gitlab::Redis::MultiStore do
       end
 
       it 'increment read fallback count metrics' do
-        expect(multi_store).to receive(:increment_read_fallback_count).with(name)
+        expect(counter).to receive(:increment).with(command: name, instance_name: instance_name)
 
         subject
       end
@@ -401,9 +407,12 @@ RSpec.describe Gitlab::Redis::MultiStore do
   end
 
   context 'with unsupported command' do
+    let(:counter) { Gitlab::Metrics::NullMetric.instance }
+
     before do
       primary_store.flushdb
       secondary_store.flushdb
+      allow(Gitlab::Metrics).to receive(:counter).and_return(counter)
     end
 
     let_it_be(:key) { "redis:counter" }
@@ -426,7 +435,7 @@ RSpec.describe Gitlab::Redis::MultiStore do
     end
 
     it 'increments method missing counter' do
-      expect(multi_store).to receive(:increment_method_missing_count).with(:incr)
+      expect(counter).to receive(:increment).with(command: :incr, instance_name: instance_name)
 
       subject
     end

spec/lib/gitlab/redis/sessions_spec.rb

@@ -4,4 +4,54 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::Sessions do
   include_examples "redis_new_instance_shared_examples", 'sessions', Gitlab::Redis::SharedState
+
+  describe 'redis instance used in connection pool' do
+    before do
+      clear_pool
+    end
+
+    context 'when redis.sessions configuration is not provided' do
+      it 'uses ::Redis instance' do
+        expect(described_class).to receive(:config_fallback?).and_return(true)
+
+        described_class.pool.with do |redis_instance|
+          expect(redis_instance).to be_instance_of(::Redis)
+        end
+      end
+    end
+
+    context 'when redis.sessions configuration is provided' do
+      it 'instantiates an instance of MultiStore' do
+        expect(described_class).to receive(:config_fallback?).and_return(false)
+
+        described_class.pool.with do |redis_instance|
+          expect(redis_instance).to be_instance_of(::Gitlab::Redis::MultiStore)
+        end
+      end
+    end
+
+    def clear_pool
+      described_class.remove_instance_variable(:@pool)
+    rescue NameError
+      # raised if @pool was not set; ignore
+    end
+  end
+
+  describe '#store' do
+    subject { described_class.store(namespace: described_class::SESSION_NAMESPACE) }
+
+    context 'when redis.sessions configuration is provided' do
+      it 'instantiates ::Redis instance' do
+        expect(described_class).to receive(:config_fallback?).and_return(true)
+        expect(subject).to be_instance_of(::Redis::Store)
+      end
+    end
+
+    context 'when redis.sessions configuration is not provided' do
+      it 'instantiates an instance of MultiStore' do
+        expect(described_class).to receive(:config_fallback?).and_return(false)
+        expect(subject).to be_instance_of(::Gitlab::Redis::MultiStore)
+      end
+    end
+  end
 end

spec/requests/api/commits_spec.rb

@@ -377,11 +377,11 @@ RSpec.describe API::Commits do
       end
 
       context 'when using warden' do
-        it 'increments usage counters', :clean_gitlab_redis_shared_state do
+        it 'increments usage counters', :clean_gitlab_redis_sessions do
           session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d')
           session_hash = { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] }
 
-          Gitlab::Redis::SharedState.with do |redis|
+          Gitlab::Redis::Sessions.with do |redis|
             redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
           end
 

spec/support/helpers/session_helpers.rb

@@ -17,10 +17,10 @@ module SessionHelpers
   end
 
   def get_session_keys
-    Gitlab::Redis::SharedState.with { |redis| redis.scan_each(match: 'session:gitlab:*').to_a }
+    Gitlab::Redis::Sessions.with { |redis| redis.scan_each(match: 'session:gitlab:*').to_a }
   end
 
   def get_ttl(key)
-    Gitlab::Redis::SharedState.with { |redis| redis.ttl(key) }
+    Gitlab::Redis::Sessions.with { |redis| redis.ttl(key) }
   end
 end

spec/support/redis/redis_shared_examples.rb

@@ -93,18 +93,23 @@ RSpec.shared_examples "redis_shared_examples" do
     subject { described_class.new(rails_env).store }
 
     shared_examples 'redis store' do
+      let(:redis_store) { ::Redis::Store }
+      let(:redis_store_to_s) { "Redis Client connected to #{host} against DB #{redis_database}" }
+
       it 'instantiates Redis::Store' do
-        is_expected.to be_a(::Redis::Store)
-        expect(subject.to_s).to eq("Redis Client connected to #{host} against DB #{redis_database}")
+        is_expected.to be_a(redis_store)
+
+        expect(subject.to_s).to eq(redis_store_to_s)
       end
 
       context 'with the namespace' do
         let(:namespace) { 'namespace_name' }
+        let(:redis_store_to_s) { "Redis Client connected to #{host} against DB #{redis_database} with namespace #{namespace}" }
 
         subject { described_class.new(rails_env).store(namespace: namespace) }
 
         it "uses specified namespace" do
-          expect(subject.to_s).to eq("Redis Client connected to #{host} against DB #{redis_database} with namespace #{namespace}")
+          expect(subject.to_s).to eq(redis_store_to_s)
         end
       end
     end

spec/support/shared_examples/requests/api/graphql/mutations/snippets_shared_examples.rb

@@ -18,19 +18,19 @@ RSpec.shared_examples 'snippet edit usage data counters' do
     end
   end
 
-  context 'when user is not sessionless' do
+  context 'when user is not sessionless', :clean_gitlab_redis_sessions do
     before do
       session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d')
       session_hash = { 'warden.user.user.key' => [[current_user.id], current_user.encrypted_password[0, 29]] }
 
-      Gitlab::Redis::SharedState.with do |redis|
+      Gitlab::Redis::Sessions.with do |redis|
         redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
       end
 
       cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
     end
 
-    it 'tracks usage data actions', :clean_gitlab_redis_shared_state do
+    it 'tracks usage data actions', :clean_gitlab_redis_sessions do
       expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_snippet_editor_edit_action)
 
       post_graphql_mutation(mutation)

spec/tasks/gitlab/cleanup_rake_spec.rb

@@ -166,14 +166,14 @@ RSpec.describe 'gitlab:cleanup rake tasks', :silence_stdout do
   end
 
   context 'sessions' do
-    describe 'gitlab:cleanup:sessions:active_sessions_lookup_keys', :clean_gitlab_redis_shared_state do
+    describe 'gitlab:cleanup:sessions:active_sessions_lookup_keys', :clean_gitlab_redis_sessions 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|
+        Gitlab::Redis::Sessions.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)
@@ -185,7 +185,7 @@ RSpec.describe 'gitlab:cleanup rake tasks', :silence_stdout do
       end
 
       it 'removes expired active session lookup keys' do
-        Gitlab::Redis::SharedState.with do |redis|
+        Gitlab::Redis::Sessions.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(