Are clients thread-safe?

[weynsee]

I'm wondering it I can share Elasticsearch::Client instances across multiple threads, or should I instantiate a new one per request/thread?

[karmi]

In my experience it's quite hard to prove something is threadsafe, as opposed to proving it isn't.

The library itself and every Elasticsearch::Client instance should be 100% threadsafe, but no formal effort has been undertaken to prove or test it.

[weynsee]

Thanks! I'll start off with the assumption that the Client object can be shared across instances and I'll just report if any issues crop up.

[karmi]

That'd be great, thanks!

[kainosnoema]

Technically no. From what I've seen in the code so far, the underlying transport is not thread-safe. In particular, Elasticsearch::Transport::Transport::Connections::Collection, which maintains the pool of connections and their state (healthy, unhealthy, etc.), does not synchronize access across threads. #get_connection does not prevent that connection from being used in another thread, and doesn't synchronize mutation to that connection's health state, so a race condition could occur if multiple threads are using the same connection.

Though I'd prefer this to be addressed, in practice as long as you're using a thread-safe HTTP transport with Faraday, you probably won't see much trouble. We're using it in Sidekiq and haven't run into anything in particular.

[ggauravuee]

Ae per comment from @kimchy - "Yes, the NodeClient is thread safe (any Client is thread safe, also the TransportClient) and its lifecycle should be similar to the application lifecycle." (http://elasticsearch-users.115913.n3.nabble.com/Is-NodeClient-thread-safe-td2816264.html)

But, I would like to know how client handles multiple requests at same time? Will requests queue up? Or does it have a connection pool implemented? Are there some configuration exposed to specify the pool size / concurrent threads etc for NodeClient java API.

Thanks

[karmi]

@ggauravuee Your question seems to be for the Java client -- the mailing list or the Github issues for the main elasticsearch repo are the best place to ask.

The Ruby client should be threadsafe in the Ruby environment.

[cheald]

@karmi "The Ruby client should be threadsafe in the Ruby environment" - is this because of the assumption of the GIL? I'm working in JRuby, which doesn't have a GIL. Is the same assumption valid?

[karmi]

@cheald No assumptions about GIL -- the statement just means that I'm not aware by any code which would flat out violate thread-safeness (class variables and such) on top of my head. The client is used in thread-safe sensitive environments (eg. Heroku) and seems to run just fine. Of course, any hypothesis like that can be proved wrong :)

[hydrogen18]

I've dug through the entire elasticsearch-transport gem and can't figure out how it could possibly be thread safe. It crashes for me when using multiple threads. My observation is the client is probably crash-free in most cases. Elasticsearch::Transport::Transport::Connections::Collection#get_connection winds up being involved in the crash. It either returns nil or tries to invoke a method on nil

I'm using Faraday under the hood. I didn't really care to debug it so I just built a subclass of Elasticsearch::Transport::Transport::HTTP::Faraday that uses a thread-safe implementation that behaves the same as a Elasticsearch::Transport::Transport::Connections::Collection but uses the MonitorMixin to achieve safety.

[kainosnoema]

@hydrogen18 this was my assessment as well (see my first post above). Doesn't look like much has changed for the better since then.

@karmi we should clarify the question, as it seems like you're discussing thread-safety in a global sense (class variables, global state), whereas I believe @hydrogen18 and I are referring to a single instance of Elasticsearch::Client being shared across multiple threads. The first case is probably fine from my brief persual (quite easy to attain in Ruby, actually), but isn't of much use in a multi-threaded environment where you want to have a shared connection pool. For that case (shared instance), Elasticsearch::Client is most definitely not thread-safe, as access to the data structures in the transport isn't synchronized in any way. If used concurrently from multiple threads, eventually something will go wrong.

[hydrogen18]

Simple demo on JRuby 1.17.19 and elasticsearch-ruby 1.0.12. The failures on one thread are visible without any protection in another.

require 'elasticsearch'

client = Elasticsearch::Transport::Client.new({hosts: 'http://notarealhost:9200'})

puts client.transport.connections.size

t = Thread.new do 
    while true
        if  client.transport.connections.get_connection.failures != 0
            raise 'time space continuum torn'
        end
        Kernel.sleep(0.1)
    end
end



begin
    client.indices.status
rescue StandardError => e 
    puts e.class.name.to_s
    puts e.message
end
Kernel.sleep(1.0)
puts client.transport.connections.size
puts t.join

My output

[ericu-destroyer-of-worlds] community$ ruby ~/es_transport_not_thread_safe.rb 
1
Faraday::Error::ConnectionFailed
initialize: name or service not known
1
RuntimeError: time space continuum torn
  (root) at /home/ericu/es_transport_not_thread_safe.rb:10

[kainosnoema]

@hydrogen18 unless I'm missing something, your example is actually to be expected I believe—even if the client was thread-safe. As long as the view of the transport's connections and their state is consistent across threads (in this case, one failure of a non-existent host), things are working properly. What you wouldn't want to see is an inconsistent state across threads, and that's actually a little tricky to reproduce since it requires concurrent mutation of that state.

[hydrogen18]

My demonstration shows that instances of Elasticsearch::Transport::Transport::Connections::Connection are obviously shared across threads. Even the most cursory inspection of that class reveals that class is not thread safe.

[kainosnoema]

I don't see any problem with connections being shared across threads, since Faraday is thread-safe—in fact, that's kind of the point of a shared instance, and is what you want. The only potential thread-safety issue I see is with the connection failure state of the pool being inconsistent across threads, but that would only affect certain failure scenarios.

Can you give a bit more detail on the problem are you running into? You mentioned something attempting to invoke a method on nil?

[hydrogen18]

I've never seen any documentation indicating that Faraday is thread safe. Could you point me in the direction of what you are referring to?

[cheald]

The demonstration above doesn't really indicate lack of thread-safety, simply that the same connection can be handed to multiple threads at the same time. As long as the connection itself is thread-safe, things should be fine. I can't vouch for Faraday's thread-safety though.

If you're on JRuby, then you should consider the Manticore transport instead - it is explicitly threadsafe.

Manticore has a Faraday adapter, as well, and while the adapter itself should be threadsafe, I can't say whether Faraday itself is. You might try that as a drop-in replacement though, and see how that works.

[lawrencepit]

Consider this code from Transport::Connections::Selector::RoundRobin

            def select(options={})
              # On Ruby 1.9, Array#rotate could be used instead
              @current = @current.nil? ? 0 : @current+1
              @current = 0 if @current >= connections.size
              connections[@current]
            end

suppose there is only 1 connection in the connections array.

thread 1 does: @current = @current.nil? ? 0 : @current+1
@current is now 0
thread 1 does: @current = 0 if @current >= connections.size
@current is now 0
thread 2 does: @current = @current.nil? ? 0 : @current+1
@current is now 1
thread 1 does: connections[@current]
which returns nil.
which is not correct, leading to NPEs in perform_request.