Handle or forbid putting of closed connections into a pool #33
Comments
|
May be related to TNT-24. |
If the connection belongs to a connection pool, it must be returned to the pool when calling "close" without actually closing the connection. All connections will be closed when poll:close() will be called. The same behavior is used in JDBC connection pool. With this change, an attempt to merge an unusable connection is no longer valid. But before it was valid, so now it's just a NOP. Fixed #33
If the connection belongs to a connection pool, it must be returned to the pool when calling "close" without actually closing the connection. All connections will be closed when poll:close() will be called. The same behavior is used in JDBC connection pool. With this change, an attempt to merge an unusable connection is no longer valid. But before it was valid, so now it's just a NOP. Fixed #33
If the connection belongs to a connection pool, it must be returned to the pool when calling "close" without actually closing the connection. All connections will be closed when poll:close() will be called. The same behavior is used in JDBC connection pool. With this change, an attempt to merge an unusable connection is no longer valid. But before it was valid, so now it's just a NOP. Fixed #33
|
Alternative: Just remove |
Backward compatibility will be broken (and strange decision, IMHO). Now the behavior of the connection in the case of "close" is close to the same as in the "JDBC Connection Pool". |
I disagree. It does not work correctly now. |
Why? As far as I remember, I can take a connection from the pool, close it and put it in the pool now. And this will be a valid code. |
So why this issue exists? |
After some discussion, we decided to leave the close method for connection from the pool and add an exception throwing from the pool: put () method in case the connection has been closed earlier (close is synonymous with put in this context). |
The code in the internal issue does not use :close() (just pool:get(), conn:execute(), pool:put()), so the issue does not look related. |
If the connection belongs to a connection pool, it must be returned to the pool when calling "close" without actually closing the connection. All connections will be closed when poll:close() will be called. The same behavior is used in JDBC connection pool. With this change, an attempt to "put" an unusable connection is no longer valid. Fixed #33
If the connection belongs to a connection pool, it must be returned to the pool when calling "close" without actually closing the connection. All connections will be closed when poll:close() will be called. The same behavior is used in JDBC connection pool. With this change, an attempt to "put" an unusable connection is no longer valid. Code that doesn't test anything has been removed from the tests. Fixed #33
If the connection belongs to a connection pool, it must be returned to the pool when calling "close" without actually closing the connection. All connections will be closed when poll:close() will be called. The same behavior is used in JDBC connection pool. With this change, an attempt to "put" an unusable connection is no longer valid. Code that doesn't test anything has been removed from the tests. Fixed #33
Uh oh!
There was an error while loading. Please reload this page.
Case: a connection is acquired from a pool, then it is closed with
<connection object>:close()and then it is put back to the pool.Note: the code uses
<connection object>.usableto mark closed connections or ones that was put back to a pool.The documentation does not state that putting of closed connections is forbidden. The code don't give an error in the case and even correctly calculates that one more connection can be acquired (
<pool object>.queue:put(nil)under hood). However this connection has GC callback that will close underlying raw connection and will allow to acquire one more from the pool: here the math fails.(1) We can remove the GC callback and close the underlying connection at putting a closed connection to a pool.
(2) We however can give an error in the case: when a user attempt to put a connection with
<connection object>.usable == falsefield, do nothing and give an error. We do not distingush between closed and released (put to a pool) connections: both have<connection object>.usableset tofalse. If we'll forbid to put closed connections, then we'll also forbid to put a connection twice. This looks as good protection from mistakes.If we really need to put a manually closed connection to a pool, let's introduce one more flag to distinguish closed and released (put to a pool) connections.
I propose to implement the approach (2).
NB: Don't forget to add a test to verify the new behaviour.
The text was updated successfully, but these errors were encountered: