From 1b0c71ef8423cf20532953e58735dd7f61325e85 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 18 Jun 2019 13:50:46 -0700
Subject: [PATCH] Cache feature flag names in Redis for a minute

We saw on GitLab.com, the SQL query, `SELECT "features"."key" FROM
"features"` peaked at 2300 times per second.

We can quiet this down a bit by caching it in Redis for a minute.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/63435
---
 changelogs/unreleased/sh-cache-feature-flag-names.yml | 5 +++++
 lib/feature.rb                                        | 7 ++++++-
 spec/lib/feature_spec.rb                              | 9 ++++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/sh-cache-feature-flag-names.yml

diff --git a/changelogs/unreleased/sh-cache-feature-flag-names.yml b/changelogs/unreleased/sh-cache-feature-flag-names.yml
new file mode 100644
index 00000000000..6120c4870f8
--- /dev/null
+++ b/changelogs/unreleased/sh-cache-feature-flag-names.yml
@@ -0,0 +1,5 @@
+---
+title: Cache feature flag names in Redis for a minute
+merge_request: 29816
+author:
+type: performance
diff --git a/lib/feature.rb b/lib/feature.rb
index 749c861d740..cc9c9d44005 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -30,7 +30,12 @@ class Feature
     end
 
     def persisted_names
-      Gitlab::SafeRequestStore[:flipper_persisted_names] ||= FlipperFeature.feature_names
+      Gitlab::SafeRequestStore[:flipper_persisted_names] ||=
+        begin
+          # We saw on GitLab.com, this database request was called 2300
+          # times/s. Let's cache it for a minute to avoid that load.
+          Rails.cache.fetch('flipper:persisted_names', expires_in: 1.minute) { FlipperFeature.feature_names }
+        end
     end
 
     def persisted?(feature)
diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb
index a7163048370..6f05914f915 100644
--- a/spec/lib/feature_spec.rb
+++ b/spec/lib/feature_spec.rb
@@ -31,7 +31,8 @@ describe Feature do
       expect(described_class.persisted_names).to be_empty
     end
 
-    it 'caches the feature names when request store is active', :request_store do
+    it 'caches the feature names when request store is active',
+       :request_store, :use_clean_rails_memory_store_caching do
       Feature::FlipperFeature.create!(key: 'foo')
 
       expect(Feature::FlipperFeature)
@@ -39,6 +40,12 @@ describe Feature do
         .once
         .and_call_original
 
+      expect(Rails.cache)
+        .to receive(:fetch)
+        .once
+        .with('flipper:persisted_names', expires_in: 1.minute)
+        .and_call_original
+
       2.times do
         expect(described_class.persisted_names).to eq(%w[foo])
       end