snower / TorMySQL

The highest performance asynchronous MySQL driver by PyMySQL
MIT License
308 stars 63 forks source link

Do not increase _connections_count in case of connect error #7

Closed vleonov closed 9 years ago

vleonov commented 9 years ago

In case of connection error _connections_count is inscreased and _used_connections is appended by new connection. But, instead of returning this connection we got OperationalError. From now on there is no way to close this connection. After _max_connections tries there is no place for new connections, so we got pool, full of dead connections, with no chance to fix them, because all of them are in _used_connections. I suppose you could solve it by fixing one line:

    connection = Connection(self, *self._args, **self._kwargs)
+   connection_future = connection.connect()
    connection.set_close_callback(self.connection_close_callback)
    self._connections_count += 1
    self._used_connections[id(connection)] = connection
-   connection_future = connection.connect()
    IOLoop.current().add_future(connection_future, on_connected)
snower commented 9 years ago

yes, good job! It is fixed.

vleonov commented 9 years ago

Unfortunately, I was wrong. This change doesn't fix the problem, because asynchrony and future. The real fixing is to move self._connections_count += 1 and self._used_connections[id(connection)] = connection into on_connected callback.

snower commented 9 years ago

No, in fact if connection.connect is called, the connect timeout exception or other exception will call self.connection_close_callback, so, the problem is fixed.

vleonov commented 9 years ago

As I can see, self.connection_close_callback is not called in case of exception, because connection hasn't been open yet. So, _connections_count continue to increase everytime, but it never decrease in case of exception. Could you please fix it, or tell which settings can help me manage dead connections?

snower commented 9 years ago

The iostream will be call close_callback in case of connect exception, so I fix it. https://github.com/snower/TorMySQL/blob/master/tormysql/connections.py#L98

vleonov commented 9 years ago

Great! Thanks a lot!