Commit 5079e8f5 authored by Kamil Trzciński's avatar Kamil Trzciński

Remove `FF_LEGACY_PERSISTED_NAMES` feature flag

The new method of fetching feature flags
seems to behave correctly.

This is safe to remove the feature flag.
parent 31307c8b
...@@ -34,26 +34,13 @@ class Feature ...@@ -34,26 +34,13 @@ class Feature
def persisted_names def persisted_names
return [] unless Gitlab::Database.exists? return [] unless Gitlab::Database.exists?
if Gitlab::Utils.to_boolean(ENV['FF_LEGACY_PERSISTED_NAMES']) # This loads names of all stored feature flags
# To be removed: # and returns a stable Set in the following order:
# This uses a legacy persisted names that are know to work (always) # - Memoized: using Gitlab::SafeRequestStore or @flipper
Gitlab::SafeRequestStore[:flipper_persisted_names] ||= # - L1: using Process cache
begin # - L2: using Redis cache
# We saw on GitLab.com, this database request was called 2300 # - DB: using a single SQL query
# times/s. Let's cache it for a minute to avoid that load. flipper.adapter.features
Gitlab::ProcessMemoryCache.cache_backend.fetch('flipper:persisted_names', expires_in: 1.minute) do
FlipperFeature.feature_names
end.to_set
end
else
# This loads names of all stored feature flags
# and returns a stable Set in the following order:
# - Memoized: using Gitlab::SafeRequestStore or @flipper
# - L1: using Process cache
# - L2: using Redis cache
# - DB: using a single SQL query
flipper.adapter.features
end
end end
def persisted_name?(feature_name) def persisted_name?(feature_name)
......
...@@ -21,66 +21,29 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -21,66 +21,29 @@ RSpec.describe Feature, stub_feature_flags: false do
end end
describe '.persisted_names' do describe '.persisted_names' do
context 'when FF_LEGACY_PERSISTED_NAMES=false' do it 'returns the names of the persisted features' do
before do Feature.enable('foo')
stub_env('FF_LEGACY_PERSISTED_NAMES', 'false')
end
it 'returns the names of the persisted features' do
Feature.enable('foo')
expect(described_class.persisted_names).to contain_exactly('foo')
end
it 'returns an empty Array when no features are presisted' do
expect(described_class.persisted_names).to be_empty
end
it 'caches the feature names when request store is active', expect(described_class.persisted_names).to contain_exactly('foo')
:request_store, :use_clean_rails_memory_store_caching do end
Feature.enable('foo')
expect(Gitlab::ProcessMemoryCache.cache_backend)
.to receive(:fetch)
.once
.with('flipper/v1/features', expires_in: 1.minute)
.and_call_original
2.times do it 'returns an empty Array when no features are presisted' do
expect(described_class.persisted_names).to contain_exactly('foo') expect(described_class.persisted_names).to be_empty
end
end
end end
context 'when FF_LEGACY_PERSISTED_NAMES=true' do it 'caches the feature names when request store is active',
before do :request_store, :use_clean_rails_memory_store_caching do
stub_env('FF_LEGACY_PERSISTED_NAMES', 'true') Feature.enable('foo')
end
it 'returns the names of the persisted features' do expect(Gitlab::ProcessMemoryCache.cache_backend)
Feature.enable('foo') .to receive(:fetch)
.once
.with('flipper/v1/features', expires_in: 1.minute)
.and_call_original
2.times do
expect(described_class.persisted_names).to contain_exactly('foo') expect(described_class.persisted_names).to contain_exactly('foo')
end end
it 'returns an empty Array when no features are presisted' do
expect(described_class.persisted_names).to be_empty
end
it 'caches the feature names when request store is active',
:request_store, :use_clean_rails_memory_store_caching do
Feature.enable('foo')
expect(Gitlab::ProcessMemoryCache.cache_backend)
.to receive(:fetch)
.once
.with('flipper:persisted_names', expires_in: 1.minute)
.and_call_original
2.times do
expect(described_class.persisted_names).to contain_exactly('foo')
end
end
end end
it 'fetches all flags once in a single query', :request_store do it 'fetches all flags once in a single query', :request_store do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment