Use the ShardedThreadedConnectionPool to disconnect after policy load #2584
Conversation
This file contains hidden or 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
![qlty-cloud-legacy[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
micahlee
suggested changes
Jun 24, 2022
ajay-vel
force-pushed
the
fix-postgres-memory-usage
branch
from
da207fa to
29c5d25
Compare
micahlee
reviewed
Jun 27, 2022
Contributor
micahlee
left a comment
micahlee
left a comment
There was a problem hiding this comment.
This a couple of further follow ups. Also, I'm not entirely sure if it was accidental or intentional, but we should spell out 'policy' in the commit message (plcy -> policy).
If you're trying to use a shorter message, I would suggest changing the headline and adding a body like:
Disconnect the database connection after policy loads
... Add more detail about why (Postgres's catcache) and how (with the shardedconnection pool) in the commit message body...
9aeb86e to
55f247f
Compare
PostgreSQL has many types of caches, one of which is the "catalog cache".When a connection is established to the database, this cache is initialized alongside it, and persists for the duration of the connection. This cache contains references to Database Objects, such as indexes, etc. (not data records themselves).This cache is not cleaned up by the system automatically. However, if the connection is disconnected, the cache is dumped. Further reading: Postgres community email thread: https://www.postgresql.org/message-id/flat/20161219.201505.11562604.horiguchi.kyotaro@lab.ntt.co.jp. The default connection pool does not support closing connections.We must be able to close connections on demand, to clear the connection cache after policy loads. The ShardedThreadedConnectionPool does support closing connections on-demand [docs](https://www.rubydoc.info/github/jeremyevans/sequel/Sequel/ShardedThreadedConnectionPool)
ajay-vel
force-pushed
the
fix-postgres-memory-usage
branch
from
55f247f to
af95002
Compare
micahlee
approved these changes
Jun 29, 2022
|
Code Climate has analyzed commit af95002 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 89.8%. View more on Code Climate. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Desired Outcome
The desired outcome of this PR is to resolve the memory leak in the postgres container causing it to OOM.
Implemented Changes
This PR:
Makes use of ShardedThreadedConnectionPool in Sequel so that active connections can be closed.
Closing active connections from conjur ensures that memory consumption in postgres stays low and a new connection for conjur is established every-time on policy load instead of using the existing connection.
- Connected Issue/Story
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
Behavior
Security