From cae3417381222c527077be5330ef1b2222d31103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 26 Sep 2017 19:40:04 -0300 Subject: Don't enforce gitaly request limits for distinct calls --- lib/gitlab/gitaly_client.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 955d2307f88..3f10951f49e 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -165,7 +165,13 @@ module Gitlab return if permitted_call_count <= MAXIMUM_GITALY_CALLS - raise TooManyInvocationsError.new(call_site, actual_call_count, max_call_count, max_stacks) + # We've exceeded the limit, but we may still be in the presence of a non + # n+1 but still complex request with many distinct calls. If the maximum + # call count is 1 or less that's probably the case. + max_count = max_call_count + return if max_count <= 1 + + raise TooManyInvocationsError.new(call_site, actual_call_count, max_count, max_stacks) end def self.allow_n_plus_1_calls -- cgit v1.2.3 From a02881dfda8cc65cfa5f9eeeee5f2c24496cbb69 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 26 Sep 2017 20:07:27 +0200 Subject: RepositoryExists is always called with #gitaly_migration --- app/models/repository.rb | 18 +----------------- changelogs/unreleased/zj-repo-gitaly.yml | 5 +++++ lib/gitlab/git/repository.rb | 14 ++++++++++++-- 3 files changed, 18 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/zj-repo-gitaly.yml diff --git a/app/models/repository.rb b/app/models/repository.rb index b28fe79e19c..f0de2697dfc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -489,13 +489,7 @@ class Repository def exists? return false unless full_path - Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| - if enabled - raw_repository.exists? - else - refs_directory_exists? - end - end + raw_repository.exists? end cache_method :exists? @@ -1063,12 +1057,6 @@ class Repository blob.data end - def refs_directory_exists? - circuit_breaker.perform do - File.exist?(File.join(path_to_repo, 'refs')) - end - end - def cache # TODO: should we use UUIDs here? We could move repositories without clearing this cache @cache ||= RepositoryCache.new(full_path, @project.id) @@ -1120,10 +1108,6 @@ class Repository Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, false)) end - def circuit_breaker - @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(project.repository_storage) - end - def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) ref ||= root_ref diff --git a/changelogs/unreleased/zj-repo-gitaly.yml b/changelogs/unreleased/zj-repo-gitaly.yml new file mode 100644 index 00000000000..634f6ba1b8b --- /dev/null +++ b/changelogs/unreleased/zj-repo-gitaly.yml @@ -0,0 +1,5 @@ +--- +title: Gitaly RepositoryExists remains opt-in for all method calls +merge_request: +author: +type: fixed diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 616b075c087..8c1df650567 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -73,8 +73,6 @@ module Gitlab delegate :empty?, to: :rugged - delegate :exists?, to: :gitaly_repository_client - def ==(other) path == other.path end @@ -102,6 +100,18 @@ module Gitlab @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(storage) end + def exists? + Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| + if enabled + gitaly_repository_client.exists? + else + circuit_breaker.perform do + File.exist?(File.join(@path, 'refs')) + end + end + end + end + # Returns an Array of branch names # sorted by name ASC def branch_names -- cgit v1.2.3 From 121057c52b65b1ac53bebe62312485918bdd863a Mon Sep 17 00:00:00 2001 From: Andrew Newdigate Date: Wed, 27 Sep 2017 19:28:36 +0100 Subject: Rolling back change to n+1 detection --- lib/gitlab/gitaly_client.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 3f10951f49e..955d2307f88 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -165,13 +165,7 @@ module Gitlab return if permitted_call_count <= MAXIMUM_GITALY_CALLS - # We've exceeded the limit, but we may still be in the presence of a non - # n+1 but still complex request with many distinct calls. If the maximum - # call count is 1 or less that's probably the case. - max_count = max_call_count - return if max_count <= 1 - - raise TooManyInvocationsError.new(call_site, actual_call_count, max_count, max_stacks) + raise TooManyInvocationsError.new(call_site, actual_call_count, max_call_count, max_stacks) end def self.allow_n_plus_1_calls -- cgit v1.2.3