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

Golang | Why deferring a Rollback for every transaction is both safe and a "Good Thing" #78

Open
atc0005 opened this issue Apr 22, 2020 · 0 comments

Comments

@atc0005
Copy link
Owner

atc0005 commented Apr 22, 2020

From VividCortex/go-database-sql-tutorial#85 (review):

Every transaction must end with either a Commit or Rollback. Deferring the Rollback is a safeguard to make sure it's called no matter what. The alternative is that you Rollback in every single error return. Miss one and you'll leak the transaction. Even if you diligently add it to all returns, you still have the problem of a potential panic that your application recovers later on. Deferring allows you to handle all this.

The problem of "always" running a rollback is not really an issue. It may seem counterintuitive at first, but database/sql actually guarantees that once the transaction is first closed (either by Commit or Rollback), subsequent attempts at closing it will be gracefully ignored. From the docs:

After a call to Commit or Rollback, all operations on the transaction fail with ErrTxDone.

This means that the only thing you need to worry about is the Commit when you're ready to do so. Any other path will automatically get a Rollback and, if you actually reached the Commit, the deferred Rollback will be a harmless no-op.

As for the Error versus Fatal... you're right, a typical application won't simply exit. Examples are typically a bit vague on error handling to avoid introducing complexity that's not really the point and may become a distraction. In this case the change to Error is not always correct either. If you failed to db.Begin a transaction, then you can't proceed and try to tx.Prepare. (I mean... you can, but it's not a good practice. It will also fail at best, or maybe even crash your application.) Likewise, if you couldn't prepare the stmt, then trying to Exec it is not going to work. Using log.Fatal is a simplification in terms of error handling, assuming that people can fill in the appropriate handling. We could put a return clause here instead if it helps. We need to show that this flow can't/shouldn't be continued, whether by returning or aborting the application. Changing to log.Error is actually doing a disfavor in my opinion.

@atc0005 atc0005 pinned this issue Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant