New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
database/sql: notify driver connection when returning to pool #22049
Comments
@tgulacsi @ mjibson Any comments on this proposal? |
I don't know any implications regarding Oracle. Where has this requirement appeared? Maybe @mjibson has some suggestions. |
Yes, I think this is a good proposal. It's kind of scary that we didn't have this before. Maybe session state isn't too harmful in general? In any case I like this. |
I'm hearing neutral to positive in my feedback so far (@jackc was more neutral, I'm positive, I take it from @tgulacsi it may not apply to the oracle driver, @mjibson is positive). I also like how it cleanly handles the case of notifying the pool of bad driver connections. @tgulacsi This was opened from a MS SQL Server driver issue, but I also have personal interest in it. Most of the drivers I'm aware of link session to connection, so the connection pool is really a session pool logically. This isn't really a problem, until a flaky query leave behind a temp table or something, then blows up the next time it gets run in the same session. |
cc @mattn |
Who call this Return? And this Return is called asynchronosy while driver collect rows (i.e. busy)? Then it should return ErrBusy? |
I agree Return is probably a bad name. Maybe ResetSession(...). Good point, in the above proposal I don't have good docs saying when this will be called. I'll fix to be clearer. It won't be called then the driver connection is busy with anything else, is will be called after the user code has finished the query, finished the rows, committed the Tx, or returned the dedicated connection. |
Change https://golang.org/cl/67630 mentions this issue: |
Change https://golang.org/cl/73033 mentions this issue: |
A single database connection ususally maps to a single session. A connection pool is logically also a session pool. Most sessions have a way to reset the session state which is desirable to prevent one bad query from poisoning another later query with temp table name conflicts or other persistent session resources. It also lets drivers provide users with better error messages from queryies when the underlying transport or query method fails. Internally the driver connection should now be marked as bad, but return the actual connection. When ResetSession is called on the connection it should return driver.ErrBadConn to remove it from the connection pool. Previously drivers had to choose between meaningful error messages or poisoning the connection pool. Lastly update TestPoolExhaustOnCancel from relying on a WAIT query fixing a flaky timeout issue exposed by this change. Fixes #22049 Fixes #20807 Change-Id: I2b5df6d954a38d0ad93bf1922ec16e74c827274c Reviewed-on: https://go-review.googlesource.com/73033 Run-TryBot: Daniel Theophanes <kardianos@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Connections are re-used between queries. However connection sessions are state-full and modifying that state may affect the next query. It would be beneficial to have a means of notifying the driver when a connection is being returned to the pool asynchronously to user code to allow driver connections to reset their session.
This would also allow driver connections to notify the pool that their connection state is bad, while still returning the real error to the user.
This would also fix #20807
The text was updated successfully, but these errors were encountered: