Skip to content

Remove MultiStore in Gitlab::Redis::SharedState

Sylvester Chin requested to merge sc1-remove-shared-state-multistore into master

What does this MR do and why?

This MR removes MultiStore usage in Gitlab::Redis::SharedState. It is part of the clean up in gitlab-com/gl-infra/scalability#2651 (closed) post-migration in gitlab-com/gl-infra/production#17188 (comment 1691514513)

The change may seem confusing as SharedState.pool now uses ClusterSharedState.pool which has 2 effects on different users

  • For GitLab Saas, since we have ClusterSharedState configured to connect with our new Redis Cluster, SharedState will also connect to that Redis Cluster.
  • For SM users, since they do not have ClusterSharedState configured, both ClusterSharedState and SharedState instances will connect using SharedState.params information.

Why the roundabout way?

This needs to be done to avoid further increasing the client connection count on the Redis Cluster which is at ~80% saturation ratio during peak periods.

The typical way to clean up post-migration would be to (1) reconfigure the MultiStore such that both primary and secondary stores connect to the same instance, then (2) clean up the MultiStore and the ClusterSharedState instance. However that would increase the connection count by up to 50% since the secondary store now initialises a connection. Note that Redis Cluster client initialises a connection even if un-used as it fetches cluster topology information using https://github.com/redis/redis-rb/blob/v4.8.0/lib/redis/cluster/node_loader.rb#L14.

This MR re-uses ClusterSharedState's pool, effectively halving the connection counts to the Redis Cluster.

The follow-up MR after cleaning up the configurations in our k8s-workloads project to this would be to remove ClusterSharedState and letting SharedState use its own connection pool.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

For GitLab SaaS:

  1. Configure cluster_shared_state using config/redis.yml
➜  gitlab git:(sc1-remove-shared-state-multistore) ✗ cat config/redis.yml
---
development:
  cluster_shared_state: redis://localhost:6379
  1. Start a rails console
# both instances share a pool
[1] pry(main)> Gitlab::Redis::ClusterSharedState.pool == Gitlab::Redis::SharedState.pool
=> true

# both configured to use `cluster_shared_state` details in redis.yml
[2] pry(main)> Gitlab::Redis::SharedState.with {|c| c}
=> #<Redis client v4.8.0 for redis://localhost:6379/0>
[3] pry(main)> Gitlab::Redis::ClusterSharedState.with{|c| c}
=> #<Redis client v4.8.0 for redis://localhost:6379/0>

# SharedState.params still read correctly for any dependent classes which uses it for config fallback
[4] pry(main)> Gitlab::Redis::ClusterSharedState.params
=> {:scheme=>"redis", :host=>"localhost", :port=>6379, :password=>nil, :instrumentation_class=>Gitlab::Instrumentation::Redis::ClusterSharedState}
[5] pry(main)> Gitlab::Redis::SharedState.params
=> {:instrumentation_class=>Gitlab::Instrumentation::Redis::SharedState, :path=>"/Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket", :db=>0}

For SM:

  1. Remove cluster_shared_state using config/redis.yml

  2. Start a rails console

[2] pry(main)> Gitlab::Redis::SharedState.params
=> {:instrumentation_class=>Gitlab::Instrumentation::Redis::SharedState, :path=>"/Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket", :db=>0}
[3] pry(main)> Gitlab::Redis::ClusterSharedState.params
=> {:instrumentation_class=>Gitlab::Instrumentation::Redis::ClusterSharedState, :path=>"/Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket", :db=>0}

# both instances configured in a similar manner
[4] pry(main)> Gitlab::Redis::SharedState.with {|c| c}
=> #<Redis client v4.8.0 for unix:///Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket/0>
[5] pry(main)> Gitlab::Redis::ClusterSharedState.with {|c| c}
=> #<Redis client v4.8.0 for unix:///Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket/0>
[6] pry(main)> Gitlab::Redis::ClusterSharedState.pool == Gitlab::Redis::SharedState.pool
=> true

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Sylvester Chin

Merge request reports

Loading