Avoid a memory leak in Excon #1167
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| # connection object to the Nomis 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 |
There was a problem hiding this comment.
"and those threads reusing"
There was a problem hiding this comment.
I think my version is right. @KPobeeNorris can you clarify the English grammar rules in this case?
There was a problem hiding this comment.
I believe that Alan's original comment is the correct case; just need to change the spelling of 'effectively'.
StupidCodeFactory
suggested changes
Oct 27, 2017
app/services/nomis/client.rb
Outdated
| # | ||
| # 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 (25 vs 5) |
There was a problem hiding this comment.
Can you provide more details, this isn't really clear why
alan
force-pushed
the
avoid-excon-memory-leak
branch
from
84e48fb
to
b92ddb3
Compare
alan
force-pushed
the
avoid-excon-memory-leak
branch
from
b92ddb3
to
9a176b3
Compare
alan
force-pushed
the
avoid-excon-memory-leak
branch
from
9a176b3
to
54228e2
Compare
alan
force-pushed
the
avoid-excon-memory-leak
branch
from
54228e2
to
fe67b56
Compare
Current version of Excon has a memory leak when threads using an Excon connection get destroyed. Applies same fix as ministryofjustice/prison-visits-public#346 The memory leak in pvb2 is slower because Puma is configured to use less threads so they probably get recycled less often. Also sets a constant number of threads for Puma in production as we do in pvb-public.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Current version of Excon has a memory leak when threads using an Excon
connection get destroyed.
Applies same fix as
ministryofjustice/prison-visits-public#346
The memory leak in pvb2 is slower because Puma is configured to use less threads
so they probably get recycled less often.
Also sets a constant number of threads for Puma in production as we do in
pvb-public.