This repository has been archived by the owner on Jan 9, 2024. It is now read-only.
Stop leaking connections when pool is full #473
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
When our connection pool is full (there are no available connections) we can run into a case where we leak perfectly useable connections. This essentially reduces the size of our connection pool every time it happens, as it can slowly leak away our unused connections until we don't have enough connections available to cope with our workload. This is particularly problematic when preparing connections for a pipeline, since we can potentially request (and leak) a large number of connections in one go. This change makes sure that before raising an exception if there are no available connections we return back all connections we've checked to the pool. In cases where we can't guarantee the connections we'd return to the pool are in a safe state we just add back `None` objects to the pool so at least the effective size of the connection pool remains unchanged.
| # If we don't add it back to the pool it shouldn't count towards the | ||
| # connection pool, or we'll artificially reduce the maximum size of the | ||
| # pool | ||
| self._created_connections_per_node.setdefault(node['name'], 0) |
There was a problem hiding this comment.
Suggested change
| self._created_connections_per_node.setdefault(node['name'], 0) | |
| self._created_connections_per_node.setdefault(connection.node['name'], 0) |
| try: | ||
| nodes[node_name] = NodeCommands(self.parse_response, self.connection_pool.get_connection_by_node(node)) | ||
| except: | ||
| # Sommething happened, maybe the pool is full, we need to release any connection |
There was a problem hiding this comment.
typo:
Suggested change
| # Sommething happened, maybe the pool is full, we need to release any connection | |
| # Something happened, maybe the pool is full, we need to release any connection |
jerroydmoore
reviewed
Sep 7, 2021
| """ | ||
| Releases the connection back to the pool | ||
| """ | ||
| self._checkpid() | ||
| if connection.pid != self.pid: | ||
| return | ||
|
|
||
| # In some cases we don't want to add back this connection to the pool but | ||
| # we still want to free its slot |
There was a problem hiding this comment.
Suggested change
| # we still want to free its slot | |
| # we still want to free its slot (a None represents an available connection | |
| # pool slot) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #458
When our connection pool is full (there are no available connections) we
can run into a case where we leak perfectly useable connections. This
essentially reduces the size of our connection pool every time it
happens, as it can slowly leak away our unused connections until we
don't have enough connections available to cope with our workload.
This is particularly problematic when preparing connections for a
pipeline, since we can potentially request (and leak) a large number of
connections in one go.
This change makes sure that before raising an exception if there are no
available connections we return back all connections we've checked to
the pool. In cases where we can't guarantee the connections we'd return
to the pool are in a safe state we just add back
Noneobjects to thepool so at least the effective size of the connection pool remains
unchanged.