Draft: Add FeatureFlag singleton to avoid cyclic deps
What does this MR do and why?
This MR logically partitions feature-flag operations from the rest of redis-cache
. It allows the use of MultiStore
in Rails.cache without the cyclical dependency found in gitlab-com/gl-infra/scalability#1992 (comment 1163520195).
The downside is that the number of connected clients to redis-cache
will increase (up to double).
See gitlab-com/gl-infra/scalability#1994 (comment 1165591557) and gitlab-com/gl-infra/scalability#2323 (closed)
Note that runbook needs to be updated since a new label storage: feature_flag
will be introduced.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
- Add multistore into cache (just to trigger some diffs)
diff --git a/lib/gitlab/redis/cache.rb b/lib/gitlab/redis/cache.rb
index ba3af3e7a6f9..13a105194502 100644
--- a/lib/gitlab/redis/cache.rb
+++ b/lib/gitlab/redis/cache.rb
@@ -19,6 +19,15 @@ def self.active_support_config
def self.default_ttl_seconds
ENV.fetch('GITLAB_RAILS_CACHE_DEFAULT_TTL_SECONDS', 8.hours).to_i
end
+
+ class << self
+ def redis
+ primary_store = ::Redis.new(::Gitlab::Redis::ClusterRateLimiting.params)
+ secondary_store = ::Redis.new(params)
+
+ MultiStore.new(primary_store, secondary_store, name.demodulize)
+ end
+ end
end
end
Also run ./bin/feature-flag use_primary_and_secondary_stores_for_cache
and ./bin/feature-flag use_primary_store_as_default_for_cache
to add feature flags.
- On master branch, running
gdk rails c
would give
➜ gitlab git:(master) ✗ gdk rails c
WARNING: This version of GitLab depends on gitlab-shell 14.17.0, but you're running 14.15.0. Please update gitlab-shell.
/Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.2/lib/active_record/core.rb:263:in `connected_to_stack': stack level too deep (SystemStackError)
from /Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.2/lib/active_record/core.rb:212:in `current_role'
from /Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.2/lib/active_record/connection_handling.rb:327:in `retrieve_connection'
from /Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.2/lib/active_record/connection_handling.rb:283:in `connection'
from /Users/sylvesterchin/work/gitlab-development-kit/gitlab/lib/gitlab/database/reflection.rb:144:in `connection'
from /Users/sylvesterchin/work/gitlab-development-kit/gitlab/lib/gitlab/database/reflection.rb:94:in `exists?'
from /Users/sylvesterchin/work/gitlab-development-kit/gitlab/lib/feature.rb:273:in `unsafe_get'
from /Users/sylvesterchin/work/gitlab-development-kit/gitlab/lib/feature.rb:264:in `with_feature'
from /Users/sylvesterchin/work/gitlab-development-kit/gitlab/lib/feature.rb:246:in `current_feature_value'
... 10177 levels...
from /Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/railties-6.1.7.2/lib/rails/commands.rb:18:in `<main>'
from /Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
from /Users/sylvesterchin/.asdf/installs/ruby/3.0.5/lib/ruby/gems/3.0.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
from bin/rails:4:in `<main>'
- On this branch, gdk would start up
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.