Skip to content

Conversation

@zhenlineo
Copy link
Contributor

Do not purge inUse connections when connection error happens or new routing table is available

Instead we deactive the server (a.k.a. closing all idle connections) in connection pool.

When an connection failure happens, the connection server will be removed from the routing table and all idle connections with this server will be closed in the connection pool.

When a new routing table is available, all idle connections towards the servers that are not in the new routing table will be removed. If the server has no inUse connections, then this server will also be totally removed from the connection pool.

Note:

  • There is no extra protection to against acquire to create a new connection with an inactive server except that the server should not be in the routing table and therefore should not be used to acquire new connections.
  • When error happens on a connection, we will deactivate the server but we will probably not remove all server's connections from connection pool as the failed connection is highly still inUse by the broken session.

@zhenlineo zhenlineo requested a review from technige November 23, 2017 16:24
@zhenlineo zhenlineo force-pushed the 1.5-purge-idle branch 4 times, most recently from b1cd45c to 0abb9f8 Compare November 24, 2017 12:52
@zhenlineo zhenlineo changed the base branch from 1.6 to 1.5 November 27, 2017 11:17
self.ttl = new_routing_table.ttl

def servers(self):
return set(self.routers.elements() + self.writers.elements() + self.readers.elements())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be return set(self.routers) | set(self.writers) | set(self.readers) and there is no need to add an elements accessor on OrderedSet.

e.update(OrderedDict.fromkeys(elements))

def elements(self):
return list(self._elements)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed - see below.

conn.close()
except IOError:
pass
if len(connections) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not connections

Zhen added 2 commits November 29, 2017 13:44
…outing table is available

Instead we deactive the server (a.k.a. closing all idle connections) in connection pool.

When an connection failure happens, the connection server will be removed from the routing table and all idle connections with this server will be closed in the connection pool.
When a new routing table is available, all idle connections towards the servers that are not in the new routing table will be removed. If the server has no inUse connections, then this server will also be totally removed from the connection pool.

Note: there is no extra protection to against `acquire` to create a new connection with an inactive server except that the server should not be in the routing table and therefore should not be used to `acquire` new connections.
When error happens on a connection, we will deactivate the server but we will probably not remove all server's connections from connection pool as the failed connection is highly still inUse by the broken session.
@zhenlineo zhenlineo merged commit ad928c5 into neo4j:1.5 Nov 29, 2017
@zhenlineo zhenlineo deleted the 1.5-purge-idle branch November 29, 2017 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants