diff options
author | Jacob Vosmaer <jacob@gitlab.com> | 2017-05-10 15:18:59 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2017-05-15 11:52:33 +0300 |
commit | 43f037c903605b55ca1c34a24ab1ea1a522816fb (patch) | |
tree | b98ac27a936432372c4833c3c9a53be88369cea7 /lib | |
parent | e261b4b8517ba6d5d5b082f1955836c945fd51fc (diff) |
Don't reuse gRPC channels
It seems that bad things happen when two gRPC stubs share one gRPC
channel so let's stop doing that. The downside of this is that we
create more gRPC connections; one per stub.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/gitlab/git/repository.rb | 14 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client.rb | 64 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/commit.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/notifications.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/ref.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/workhorse.rb | 2 |
6 files changed, 36 insertions, 52 deletions
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 256318cb833..7a051444603 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -27,13 +27,15 @@ module Gitlab # Rugged repo object attr_reader :rugged + attr_reader :storage + # 'path' must be the path to a _bare_ git repository, e.g. # /path/to/my-repo.git - def initialize(repository_storage, relative_path) - @repository_storage = repository_storage + def initialize(storage, relative_path) + @storage = storage @relative_path = relative_path - storage_path = Gitlab.config.repositories.storages[@repository_storage]['path'] + storage_path = Gitlab.config.repositories.storages[@storage]['path'] @path = File.join(storage_path, @relative_path) @name = @relative_path.split("/").last @attributes = Gitlab::Git::Attributes.new(path) @@ -965,11 +967,7 @@ module Gitlab end def gitaly_repository - Gitlab::GitalyClient::Util.repository(@repository_storage, @relative_path) - end - - def gitaly_channel - Gitlab::GitalyClient.get_channel(@repository_storage) + Gitlab::GitalyClient::Util.repository(@storage, @relative_path) end private diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c69676a1dac..72466700c05 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -4,56 +4,42 @@ module Gitlab module GitalyClient SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze - # This function is not thread-safe because it updates Hashes in instance variables. - def self.configure_channels - @addresses = {} - @channels = {} - Gitlab.config.repositories.storages.each do |name, params| - address = params['gitaly_address'] - unless address.present? - raise "storage #{name.inspect} is missing a gitaly_address" - end + MUTEX = Mutex.new + private_constant :MUTEX - unless URI(address).scheme.in?(%w(tcp unix)) - raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'" + def self.stub(name, storage) + MUTEX.synchronize do + @stubs ||= {} + @stubs[storage] ||= {} + @stubs[storage][name] ||= begin + klass = Gitaly.const_get(name.to_s.camelcase.to_sym).const_get(:Stub) + addr = address(storage) + addr = addr.sub(%r{^tcp://}, '') if URI(addr).scheme == 'tcp' + klass.new(addr, :this_channel_is_insecure) end - - @addresses[name] = address - @channels[name] = new_channel(address) end end - def self.new_channel(address) - address = address.sub(%r{^tcp://}, '') if URI(address).scheme == 'tcp' - # NOTE: When Gitaly runs on a Unix socket, permissions are - # handled using the file system and no additional authentication is - # required (therefore the :this_channel_is_insecure flag) - # TODO: Add authentication support when Gitaly is running on a TCP socket. - GRPC::Core::Channel.new(address, {}, :this_channel_is_insecure) + def self.clear_stubs! + MUTEX.synchronize do + @stubs = nil + end end - def self.get_channel(storage) - if !Rails.env.production? && @channels.nil? - # In development mode the Rails auto-loader may reset the instance - # variables of this class. What we do here is not thread-safe. In normal - # circumstances (including production) these instance variables have - # been initialized from config/initializers. - configure_channels - end + def self.address(storage) + params = Gitlab.config.repositories.storages[storage] + raise "storage not found: #{storage.inspect}" if params.nil? - @channels[storage] - end + address = params['gitaly_address'] + unless address.present? + raise "storage #{storage.inspect} is missing a gitaly_address" + end - def self.get_address(storage) - if !Rails.env.production? && @addresses.nil? - # In development mode the Rails auto-loader may reset the instance - # variables of this class. What we do here is not thread-safe. In normal - # circumstances (including development) these instance variables have - # been initialized from config/initializers. - configure_channels + unless URI(address).scheme.in?(%w(tcp unix)) + raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'" end - @addresses[storage] + address end def self.enabled? diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb index 15c57420fb2..4491903d788 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -11,7 +11,7 @@ module Gitlab end def is_ancestor(ancestor_id, child_id) - stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: @repository.gitaly_channel) + stub = GitalyClient.stub(:commit, @repository.storage) request = Gitaly::CommitIsAncestorRequest.new( repository: @gitaly_repo, ancestor_id: ancestor_id, @@ -52,7 +52,7 @@ module Gitlab end def diff_service_stub - Gitaly::Diff::Stub.new(nil, nil, channel_override: @repository.gitaly_channel) + GitalyClient.stub(:diff, @repository.storage) end end end diff --git a/lib/gitlab/gitaly_client/notifications.rb b/lib/gitlab/gitaly_client/notifications.rb index a94a54883db..719554eac52 100644 --- a/lib/gitlab/gitaly_client/notifications.rb +++ b/lib/gitlab/gitaly_client/notifications.rb @@ -6,7 +6,7 @@ module Gitlab # 'repository' is a Gitlab::Git::Repository def initialize(repository) @gitaly_repo = repository.gitaly_repository - @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: repository.gitaly_channel) + @stub = GitalyClient.stub(:notifications, repository.storage) end def post_receive diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index f6c77ef1a3e..bf04e1fa50b 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -6,7 +6,7 @@ module Gitlab # 'repository' is a Gitlab::Git::Repository def initialize(repository) @gitaly_repo = repository.gitaly_repository - @stub = Gitaly::Ref::Stub.new(nil, nil, channel_override: repository.gitaly_channel) + @stub = GitalyClient.stub(:ref, repository.storage) end def default_branch_name diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 351e2b10595..fe37e4da94f 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -26,7 +26,7 @@ module Gitlab } if Gitlab.config.gitaly.enabled - address = Gitlab::GitalyClient.get_address(project.repository_storage) + address = Gitlab::GitalyClient.address(project.repository_storage) params[:Repository] = repository.gitaly_repository.to_h feature_enabled = case action.to_s |