Skip to content

Draft: Add FeatureFlag singleton to avoid cyclic deps

Sylvester Chin requested to merge sc1-add-ff-singleton into master

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

  1. 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.

  1. 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>'
  1. 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.

Edited by Sylvester Chin

Merge request reports