Skip to content
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: update documentation to explain context cancellation in more detail #27962

Closed
pjebs opened this issue Oct 2, 2018 · 7 comments
Closed
Assignees

Comments

@pjebs
Copy link
Contributor

@pjebs pjebs commented Oct 2, 2018

The documentation states in various places:

Drivers that do not support context cancelation will not return until after the query is completed.

The provided context is used until the transaction is committed or rolled back. If the context is canceled, the sql package will roll back the transaction. Tx.Commit will return an error if the context provided to BeginTx is canceled.

etc etc

@pjebs
Copy link
Contributor Author

@pjebs pjebs commented Oct 2, 2018

My suggestion is the documentation makes clear that some drivers will cancel the execution of the query whilst others will only cancel waiting for the query results but the execution will still complete behind the scenes.

This is a very important distinction.

@pjebs
Copy link
Contributor Author

@pjebs pjebs commented Oct 2, 2018

See: go-sql-driver/mysql#863 for more details.
@methane is one of the key contributors to the popular mysql driver.

@dmitshur dmitshur changed the title [database/sql]: Update documentation to explain context cancellation in more detail database/sql: update documentation to explain context cancellation in more detail Oct 3, 2018
@pjebs
Copy link
Contributor Author

@pjebs pjebs commented Oct 8, 2018

I found out this issue is more serious than a mere "documentation issue" forewarning the pitfalls.

See: go-sql-driver/mysql#863 (comment)

I have created an "empty" proposal: #28074 for auditing purposes.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 8, 2018

I'll look into more detail in the documentation.

@kardianos kardianos self-assigned this Oct 8, 2018
@methane
Copy link
Contributor

@methane methane commented Oct 9, 2018

My suggestion is the documentation makes clear that some drivers will cancel the execution of the query whilst others will only cancel waiting for the query results but the execution will still complete behind the scenes.

This is a very important distinction.

Why is it very important?

Even though driver support cancelling query on the server, it only saves some CPU usage.

Canceling remote query is hard and nondeterministic problem. You shouldn't rely on behaviour of canceling.
In case of go-sql-driver/mysql#863, you should use transaction.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 27, 2018

There is already really clear documentation. At the top of the page of https://golang.org/pkg/database/sql/

Drivers that do not support context cancelation will not return until after the query is completed.

I think that is clear enough. Closing.

@kardianos kardianos closed this Oct 27, 2018
@pjebs
Copy link
Contributor Author

@pjebs pjebs commented Oct 27, 2018

Some drivers such as MySQL driver do support cancellation.

I was trying to make clear that even if the driver does support cancellation, the documentation should warn that successfully cancelling from Go's point of view does not imply that the query is cancelled from databases point of view.

In the case of MySQL, cancelling does not mean that MySQL has cancelled the execution of the query.

@golang golang locked and limited conversation to collaborators Oct 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.