Merge branch '3650-persist-elasticsearch-client' into 'master'

Reduce the number of Elasticsearch client instances that are created

Closes #3650

See merge request gitlab-org/gitlab-ee!3432

changelogs/unreleased-ee/3650-persist-elasticsearch-client.yml

+---
+title: Reduce the number of Elasticsearch client instances that are created
+merge_request: 3432
+author:
+type: fixed

config/initializers/elastic_client_setup.rb

@@ -5,20 +5,38 @@ require 'gitlab/current_settings'
 module Elasticsearch
   module Model
     module Client
+      # This mutex is only used to synchronize *creation* of a new client, so
+      # all including classes can share the same client instance
+      CLIENT_MUTEX = Mutex.new
+
+      cattr_accessor :cached_client
+      cattr_accessor :cached_config
+
       module ClassMethods
         include Gitlab::CurrentSettings
 
-        def client(client = nil)
-          if @client.nil? || es_configuration_changed?
-            @es_config = current_application_settings.elasticsearch_config
-            @client = ::Gitlab::Elastic::Client.build(@es_config)
-          end
+        # Override the default ::Elasticsearch::Model::Client implementation to
+        # return a client configured from application settings. All including
+        # classes will use the same instance, which is refreshed automatically
+        # if the settings change.
+        #
+        # _client is present to match the arity of the overridden method, where
+        # it is also not used.
+        #
+        # @return [Elasticsearch::Transport::Client]
+        def client(_client = nil)
+          store = ::Elasticsearch::Model::Client
 
-          @client
+          store::CLIENT_MUTEX.synchronize do
+            config = current_application_settings.elasticsearch_config
+
+            if store.cached_client.nil? || config != store.cached_config
+              store.cached_client = ::Gitlab::Elastic::Client.build(config)
+              store.cached_config = config
+            end
           end
 
-        def es_configuration_changed?
-          @es_config != current_application_settings.elasticsearch_config
+          store.cached_client
         end
       end
     end

spec/elastic_integration/elasticsearch_model_client_spec.rb

+require 'spec_helper'
+
+# This module is monkey-patched in config/initializers/elastic_client_setup.rb
+describe "Monkey-patches to ::Elasticsearch::Model::Client" do
+  before do
+    stub_application_setting(elasticsearch_url: ['http://localhost:9200'])
+  end
+
+  it 'uses the same client instance for all subclasses' do
+    a = Class.new { include ::Elasticsearch::Model }
+    b = Class.new { include ::Elasticsearch::Model }
+    c = Class.new(b)
+
+    expect(::Gitlab::Elastic::Client).to receive(:build).with(anything) { :fake_client }.once
+
+    # Ensure that the same client instance is used between classes and between
+    # requests
+    [a, b, c, b, c, b, a].each do |klass|
+      expect(klass.__elasticsearch__.client).to eq(:fake_client)
+    end
+  end
+end