Send a cache invalidation event toggling features

We use the flipper gem to provide feature gates in GitLab backed
with the ActiveSupportCacheStore adapter.

In a Geo setup, the primary and secondary do not share Redis state.
This means that changes to a feature on the primary do not invalidate
the cache on the secondary, leading to inconsistent behaviour.

To fix this send a cache invalidation event via the Geo log cursor
whenever features are changed.
parent 04ae1247
......@@ -41,7 +41,7 @@ sudo -u git -H bin/rails console RAILS_ENV=production
**To check if automatic background verification is enabled:**
```ruby
Feature.enabled?('geo_repository_verification')
Gitlab::Geo.repository_verification_enabled?
```
**To disable automatic background verification:**
......@@ -56,11 +56,6 @@ Feature.disable('geo_repository_verification')
Feature.enable('geo_repository_verification')
```
NOTE: **Note:**
Until [issue #5699][ee-5699] is completed, we need to reset the cache for this
feature flag on each **secondary**, to do this run
`sudo gitlab-rails runner 'Rails.cache.expire('flipper/v1/feature/geo_repository_verification', 0)'`.
# Repository verification
Navigate to the **Admin Area > Geo** dashboard on the **primary** node and expand
......
......@@ -11,7 +11,7 @@ class Groups::PipelineQuotaController < Groups::ApplicationController
private
def all_projects
if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
if ::Feature.enabled?(:shared_runner_minutes_on_root_namespace)
@group.all_projects
else
@group.projects
......
......@@ -116,7 +116,7 @@ module EE
def shared_runner_minutes_supported?
if has_parent?
!Feature.enabled?(:shared_runner_minutes_on_root_namespace)
!::Feature.enabled?(:shared_runner_minutes_on_root_namespace)
else
true
end
......@@ -139,7 +139,7 @@ module EE
end
def shared_runners_enabled?
if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
if ::Feature.enabled?(:shared_runner_minutes_on_root_namespace)
all_projects.with_shared_runners.any?
else
projects.with_shared_runners.any?
......
......@@ -128,7 +128,7 @@ module EE
end
def shared_runners_limit_namespace
if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
if ::Feature.enabled?(:shared_runner_minutes_on_root_namespace)
root_namespace
else
namespace
......
......@@ -49,7 +49,7 @@ module EE
def all_namespaces
namespaces = ::Namespace.reorder(nil).where('namespaces.id = projects.namespace_id')
if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
if ::Feature.enabled?(:shared_runner_minutes_on_root_namespace)
namespaces = ::Gitlab::GroupHierarchy.new(namespaces).roots
end
......
# frozen_string_literal: true
module EE
module Feature
module ClassMethods
extend ::Gitlab::Utils::Override
override :enable
def enable(key, thing = true)
super
log_geo_event(key)
end
override :disable
def disable(key, thing = false)
super
log_geo_event(key)
end
override :enable_group
def enable_group(key, group)
super
log_geo_event(key)
end
override :disable_group
def disable_group(key, group)
super
log_geo_event(key)
end
private
def log_geo_event(key)
Geo::CacheInvalidationEventStore.new(cache_store.key_for(key)).create!
end
def cache_store
Flipper::Adapters::ActiveSupportCacheStore
end
end
def self.prepended(base)
base.singleton_class.prepend ClassMethods
end
end
end
......@@ -111,10 +111,10 @@ module Gitlab
end
def self.repository_verification_enabled?
feature = Feature.get('geo_repository_verification')
feature = ::Feature.get('geo_repository_verification')
# If the feature has been set, always evaluate
if Feature.persisted?(feature)
if ::Feature.persisted?(feature)
return feature.enabled?
else
true
......
# frozen_string_literal: true
require 'spec_helper'
describe Feature do
include EE::GeoHelpers
describe '.enable' do
context 'when running on a Geo primary node' do
before do
stub_primary_node
end
it 'does not create a Geo::CacheInvalidationEvent if there are no Geo secondary nodes' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [] }
expect { described_class.enable(:foo) }.not_to change(Geo::CacheInvalidationEvent, :count)
end
it 'creates a Geo::CacheInvalidationEvent' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [double] }
expect { described_class.enable(:foo) }.to change(Geo::CacheInvalidationEvent, :count).by(1)
end
end
end
describe '.disable' do
context 'when running on a Geo primary node' do
before do
stub_primary_node
end
it 'does not create a Geo::CacheInvalidationEvent if there are no Geo secondary nodes' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [] }
expect { described_class.disable(:foo) }.not_to change(Geo::CacheInvalidationEvent, :count)
end
it 'creates a Geo::CacheInvalidationEvent' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [double] }
expect { described_class.disable(:foo) }.to change(Geo::CacheInvalidationEvent, :count).by(1)
end
end
end
describe '.enable_group' do
context 'when running on a Geo primary node' do
before do
stub_primary_node
end
it 'does not create a Geo::CacheInvalidationEvent if there are no Geo secondary nodes' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [] }
expect { described_class.enable_group(:foo, :bar) }.not_to change(Geo::CacheInvalidationEvent, :count)
end
it 'creates a Geo::CacheInvalidationEvent' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [double] }
expect { described_class.enable_group(:foo, :bar) }.to change(Geo::CacheInvalidationEvent, :count).by(1)
end
end
end
describe '.disable_group' do
context 'when running on a Geo primary node' do
before do
stub_primary_node
end
it 'does not create a Geo::CacheInvalidationEvent if there are no Geo secondary nodes' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [] }
expect { described_class.disable_group(:foo, :bar) }.not_to change(Geo::CacheInvalidationEvent, :count)
end
it 'creates a Geo::CacheInvalidationEvent' do
allow(Gitlab::Geo).to receive(:secondary_nodes) { [double] }
expect { described_class.disable_group(:foo, :bar) }.to change(Geo::CacheInvalidationEvent, :count).by(1)
end
end
end
end
......@@ -2,6 +2,8 @@ require 'flipper/adapters/active_record'
require 'flipper/adapters/active_support_cache_store'
class Feature
prepend EE::Feature
# Classes to override flipper table names
class FlipperFeature < Flipper::Adapters::ActiveRecord::Feature
# Using `self.table_name` won't work. ActiveRecord bug?
......@@ -72,7 +74,11 @@ class Feature
end
def flipper
@flipper ||= (Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance)
if Gitlab::SafeRequestStore.active?
Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance
else
@flipper ||= build_flipper_instance
end
end
def build_flipper_instance
......
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