diff --git a/app/services/prison_visits/client.rb b/app/services/prison_visits/client.rb index 95457f3d6..427abc4e8 100644 --- a/app/services/prison_visits/client.rb +++ b/app/services/prison_visits/client.rb @@ -4,15 +4,34 @@ module PrisonVisits APIError = Class.new(StandardError) APINotFound = Class.new(StandardError) + # This client is **NOT** thread safe. To be used with a connection pool. See below. class Client TIMEOUT = 3 # seconds EXCON_INSTRUMENT_NAME = 'pvb_api'.freeze def initialize(host, persistent: true) @host = host + + # Sets `thread_safe_sockets` to `false` on the Excon connection. Setting + # to `false` disables Excon connection sockets cache for each thread. + # + # This cache means that there is a socket connection for every Excon + # connection object to the PVB API for each Thread. This is to make an + # Excon connection thread safe. + # + # For example if Puma runs with 4 threads, and those threads reuse a + # connection pool of 4 Excon connections means that effectevely there can + # be up to 16 live connections to the PVB API. + # + # This cache is unnecessary in our case since: + # - we ensure thread safety by using a connection pool + # - we end up opening more sockets than necessary (16 vs 4). If we only + # have 4 puma threads we only need 4 sockets + # - the cache has a memory leak when there are short lived threads. @connection = Excon.new( host, persistent: persistent, connect_timeout: TIMEOUT, read_timeout: TIMEOUT, write_timeout: TIMEOUT, retry_limit: 3, + thread_safe_sockets: false, instrumentor: ActiveSupport::Notifications, instrumentor_name: EXCON_INSTRUMENT_NAME )