Commit 4dfaaf40 authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg

Turn on Cat-File cache by default

The feature flag has been introduced an was turned off by default,
now the it will default to be turned on. That change would still allow
users to turn this feature off by leveraging the Rails console by
running:

`Feature.disable("gitaly_catfile-cache")`

Another option is to manage the number of items the LRU cache will
contain, by updating the `config.toml` for Gitaly. This would be the
`catfile_cache_size`:
https://gitlab.com/gitlab-org/gitaly/blob/0dcb5c579e63754f557aef91a4fa7a00e5b8b127/config.toml.example#L27

Closes: https://gitlab.com/gitlab-org/gitaly/issues/1712
parent 968674e4
# frozen_string_literal: true
require 'set'
class Feature class Feature
class Gitaly class Gitaly
# Server feature flags should use '_' to separate words.
# CATFILE_CACHE sets an incorrect example
CATFILE_CACHE = 'catfile-cache'.freeze CATFILE_CACHE = 'catfile-cache'.freeze
# Server feature flags should use '_' to separate words.
SERVER_FEATURE_FLAGS = [CATFILE_CACHE].freeze SERVER_FEATURE_FLAGS = [CATFILE_CACHE].freeze
DEFAULT_ON_FLAGS = Set.new([CATFILE_CACHE]).freeze
class << self class << self
def enabled?(feature_flag) def enabled?(feature_flag)
Feature::FlipperFeature.table_exists? && Feature.enabled?("gitaly_#{feature_flag}") return false unless Feature::FlipperFeature.table_exists?
default_on = DEFAULT_ON_FLAGS.include?(feature_flag)
Feature.enabled?("gitaly_#{feature_flag}", default_enabled: default_on)
rescue ActiveRecord::NoDatabaseError rescue ActiveRecord::NoDatabaseError
false false
end end
def server_feature_flags def server_feature_flags
@server_feature_flags ||=
begin
SERVER_FEATURE_FLAGS.map do |f| SERVER_FEATURE_FLAGS.map do |f|
["gitaly-feature-#{f.tr('_', '-')}", enabled?(f).to_s] ["gitaly-feature-#{f.tr('_', '-')}", enabled?(f).to_s]
end.to_h end.to_h
end end
end end
end end
end
end end
...@@ -240,7 +240,6 @@ module Gitlab ...@@ -240,7 +240,6 @@ module Gitlab
Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid
end end
def self.token(storage) def self.token(storage)
params = Gitlab.config.repositories.storages[storage] params = Gitlab.config.repositories.storages[storage]
raise "storage not found: #{storage.inspect}" if params.nil? raise "storage not found: #{storage.inspect}" if params.nil?
......
...@@ -34,7 +34,7 @@ module Gitlab ...@@ -34,7 +34,7 @@ module Gitlab
def self.disk_access_denied? def self.disk_access_denied?
return false if rugged_enabled? return false if rugged_enabled?
!temporarily_allowed?(ALLOW_KEY) && GitalyClient.feature_enabled?(DISK_ACCESS_DENIED_FLAG) !temporarily_allowed?(ALLOW_KEY) && Feature::Gitaly.enabled?(DISK_ACCESS_DENIED_FLAG)
rescue rescue
false # Err on the side of caution, don't break gitlab for people false # Err on the side of caution, don't break gitlab for people
end end
......
require 'spec_helper' require 'spec_helper'
describe Feature::Gitaly do describe Feature::Gitaly do
let(:feature_flag) { "mepmep" } let(:feature_flag) { "mep_mep" }
before do
stub_const("#{described_class}::SERVER_FEATURE_FLAGS", [feature_flag])
end
describe ".enabled?" do describe ".enabled?" do
context 'when the gate is closed' do context 'when the gate is closed' do
before do before do
allow(Feature).to receive(:enabled?).with("gitaly_mepmep").and_return(false) stub_feature_flags(gitaly_mep_mep: false)
end end
it 'returns false' do it 'returns false' do
expect(described_class.enabled?(feature_flag)).to be(false) expect(described_class.enabled?(feature_flag)).to be(false)
end end
end end
context 'when the flag defaults to on' do
it 'returns true' do
expect(described_class.enabled?(feature_flag)).to be(true)
end
end
end end
describe ".server_feature_flags" do describe ".server_feature_flags" do
context 'when one flag is disabled' do
before do before do
stub_const("#{described_class}::SERVER_FEATURE_FLAGS", [feature_flag]) stub_feature_flags(gitaly_mep_mep: false)
allow(Feature).to receive(:enabled?).with("gitaly_mepmep").and_return(false)
end end
subject { described_class.server_feature_flags } subject { described_class.server_feature_flags }
it { is_expected.to be_a(Hash) } it { is_expected.to be_a(Hash) }
it { is_expected.to eq("gitaly-feature-mep-mep" => "false") }
context 'when one flag is disabled' do
it { is_expected.to eq("gitaly-feature-mepmep" => "false") }
end end
end end
end end
...@@ -28,12 +28,16 @@ describe Gitlab::GitalyClient::StorageSettings do ...@@ -28,12 +28,16 @@ describe Gitlab::GitalyClient::StorageSettings do
end end
describe '.disk_access_denied?' do describe '.disk_access_denied?' do
it 'return false when Rugged is enabled', :enable_rugged do context 'when Rugged is enabled', :enable_rugged do
it 'returns false' do
expect(described_class.disk_access_denied?).to be_falsey expect(described_class.disk_access_denied?).to be_falsey
end end
end
context 'when Rugged is disabled' do
it 'returns true' do it 'returns true' do
expect(described_class.disk_access_denied?).to be_truthy expect(described_class.disk_access_denied?).to be_truthy
end end
end end
end
end end
...@@ -10,7 +10,7 @@ module CycleAnalyticsHelpers ...@@ -10,7 +10,7 @@ module CycleAnalyticsHelpers
repository = project.repository repository = project.repository
oldrev = repository.commit(branch_name)&.sha || Gitlab::Git::BLANK_SHA oldrev = repository.commit(branch_name)&.sha || Gitlab::Git::BLANK_SHA
if Timecop.frozen? && Gitlab::GitalyClient.feature_enabled?(:operation_user_commit_files) if Timecop.frozen?
mock_gitaly_multi_action_dates(repository, commit_time) mock_gitaly_multi_action_dates(repository, commit_time)
end end
......
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