Skip to content

Commit

Permalink
Not reuse persistent connection for healthchecks
Browse files Browse the repository at this point in the history
There is a large number of EOFError's due to the persistent connections being
inactive. For the healthcheck there is no real benefit to try to use a
persistent connection so this changes it to use a new connection.
  • Loading branch information
alan authored and StupidCodeFactory committed Oct 25, 2017
1 parent 938efa2 commit 146fff0
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 34 deletions.
12 changes: 11 additions & 1 deletion app/services/healthcheck/pvb_api_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,18 @@ class PvbApiCheck

def initialize(description)
build_report(description) do
{ ok: PrisonVisits::Api.instance.healthy? }
{ ok: healthy_pvb_connection }
end
end

private

def healthy_pvb_connection
client.healthcheck.status == 200
end

def client
PrisonVisits::Client.new(Rails.configuration.api_host, persistent: false)
end
end
end
4 changes: 0 additions & 4 deletions app/services/prison_visits/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ def initialize
end
end

def healthy?
pool.with { |client| client.healthcheck.status == 200 }
end

def get_prisons
result = pool.with { |client| client.get('/prisons') }

Expand Down
9 changes: 3 additions & 6 deletions app/services/prison_visits/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ class Client
TIMEOUT = 3 # seconds
EXCON_INSTRUMENT_NAME = 'pvb_api'.freeze

def initialize(host)
def initialize(host, persistent: true)
@host = host
@connection = Excon.new(
host, persistent: true, connect_timeout: TIMEOUT,
host, persistent: persistent, connect_timeout: TIMEOUT,
read_timeout: TIMEOUT, write_timeout: TIMEOUT, retry_limit: 3,
instrumentor: ActiveSupport::Notifications,
instrumentor_name: EXCON_INSTRUMENT_NAME
Expand All @@ -31,10 +31,7 @@ def delete(route, params: {}, idempotent: true)
end

def healthcheck
@connection.head(
path: 'healthcheck',
persistent: false
)
@connection.head(path: 'healthcheck')
end

private
Expand Down
10 changes: 6 additions & 4 deletions spec/controllers/healthcheck_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@

context 'when everything is OK' do
before do
allow_any_instance_of(PrisonVisits::Api).
to receive(:healthy?).and_return(true)
allow_any_instance_of(PrisonVisits::Client).
to receive(:healthcheck).
and_return(double(status: 200))
end

it { is_expected.to be_success }
Expand All @@ -30,8 +31,9 @@

context 'when the healthcheck is not OK' do
before do
allow_any_instance_of(PrisonVisits::Api).
to receive(:healthy?).and_return(false)
allow_any_instance_of(PrisonVisits::Client).
to receive(:healthcheck).
and_return(double(status: 502))
end

it { is_expected.to have_http_status(:bad_gateway) }
Expand Down
50 changes: 50 additions & 0 deletions spec/fixtures/vcr_cassettes/healthcheck.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions spec/services/healthcheck/pvb_api_check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

context 'with a healthy API' do
before do
allow(PrisonVisits::Api.instance).
to receive(:healthy?).
and_return(true)
allow_any_instance_of(PrisonVisits::Client).
to receive(:healthcheck).
and_return(double(status: 200))
end

it { is_expected.to be_ok }
Expand All @@ -23,8 +23,8 @@
context 'with an unhealthy API' do
context 'that raises an error' do
before do
allow(PrisonVisits::Api.instance).
to receive(:healthy?).
allow_any_instance_of(PrisonVisits::Client).
to receive(:healthcheck).
and_raise(StandardError, 'some other exception')
end

Expand All @@ -41,9 +41,9 @@

context 'that reports the status' do
before do
allow(PrisonVisits::Api.instance).
to receive(:healthy?).
and_return(false)
allow_any_instance_of(PrisonVisits::Client).
to receive(:healthcheck).
and_return(double(status: 500))
end

it { is_expected.to_not be_ok }
Expand Down
11 changes: 0 additions & 11 deletions spec/services/prison_visits/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,6 @@
end
end

describe 'healthy?', vcr: { cassette_name: 'healthy' } do
subject { super().healthy? }

it 'makes a request to the healtcheck endpoint' do
is_expected.to eq(true)

expect(WebMock).
to have_requested(:head, /healthcheck/)
end
end

describe 'get_prisons', vcr: { cassette_name: 'get_prisons' } do
subject { super().get_prisons }

Expand Down
6 changes: 6 additions & 0 deletions spec/services/prison_visits/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
RequestStore.store[:request_id] = '5cd2ef1a-4b7e-11e7-a919-92ebcb67fe33'
end

describe '#healthcheck' do
it 'calls the healthcheck endpoint', vcr: { cassette_name: 'healthcheck' } do
expect(subject.healthcheck.status). to eq(200)
end
end

describe 'error handling' do
it 'parses returned JSON errors', vcr: { cassette_name: 'client_json_error' } do
expect {
Expand Down

0 comments on commit 146fff0

Please sign in to comment.