Commit 4ea02bf8 authored by Jacob Vosmaer's avatar Jacob Vosmaer 👋
Browse files

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.
parent 6abd3c62

app/models/repository.rb

+0 −2
Original line number Diff line number Diff line
@@ -1163,8 +1163,6 @@ def repository_storage_path 
    @project.repository_storage_path

  end



  delegate :gitaly_channel, :gitaly_repository, to: :raw_repository



  def initialize_raw_repository

    Gitlab::Git::Repository.new(project.repository_storage, path_with_namespace + '.git')

  end

config/initializers/8_gitaly.rb

+4 −2
Original line number Diff line number Diff line 
require 'uri'



# Make sure we initialize our Gitaly channels before Sidekiq starts multi-threaded execution.

if Gitlab.config.gitaly.enabled || Rails.env.test?

  Gitlab::GitalyClient.configure_channels

  Gitlab.config.repositories.storages.keys.each do |storage|

    # Force validation of each address

    Gitlab::GitalyClient.address(storage)

  end

end

lib/gitlab/git/repository.rb

+6 −8
Original line number Diff line number Diff line
@@ -27,13 +27,15 @@ class Repository 
      # 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 @@ def attributes(path) 
      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

lib/gitlab/gitaly_client.rb

+25 −39
Original line number Diff line number Diff line
@@ -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



    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



      @channels[storage]

    def self.address(storage)

      params = Gitlab.config.repositories.storages[storage]

      raise "storage not found: #{storage.inspect}" if params.nil?



      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?

lib/gitlab/gitaly_client/commit.rb

+2 −2
Original line number Diff line number Diff line
@@ -9,7 +9,7 @@ class Commit 


      def initialize(repository)

        @gitaly_repo = repository.gitaly_repository

        @stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: repository.gitaly_channel)

        @stub = GitalyClient.stub(:commit, repository.storage)

      end



      def is_ancestor(ancestor_id, child_id)
@@ -26,7 +26,7 @@ class << self 
        def diff_from_parent(commit, options = {})

          repository = commit.project.repository

          gitaly_repo = repository.gitaly_repository

          stub = Gitaly::Diff::Stub.new(nil, nil, channel_override: repository.gitaly_channel)

          stub = GitalyClient.stub(:diff, repository.storage)

          parent = commit.parents[0]

          parent_id = parent ? parent.id : EMPTY_TREE_ID

          request = Gitaly::CommitDiffRequest.new(