-
-
Notifications
You must be signed in to change notification settings - Fork 731
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
Implicit transaction in DatabasePool.write and DatabaseQueue.write #332
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…don't see any changes until all changes are executed. DatabasePool.write thus wraps changes in a transaction.
…ens transactions as well...
…side a transaction
groue
changed the title
Implicit transaction in DatabasePool.write
Implicit transaction in DatabasePool.write and DatabaseQueue.write
Apr 15, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an ergonomic issue with database pools and queues, and has them wrap database accesses in a transaction by default.
The problem is clear with database pools: unless writes are wrapped in a transaction, concurrent reads see partial changes. This is positively unsafe:
This subtlety in the concurrent behavior of database pools was abundantly documented. But personal experience with coworkers and frequent feedback from GRDB users have revealed that it was too high a burden to force users to remember to use transactions in nearly all their database pool writes.
The problem is less pressing with database queues, since serialization of database accesses prevents concurrent reads from fetching inconsistent database values. Yet, personal experience again reveals that users perform a constant and conscious effort in order to choose whether to use a transaction or not, or wonder if a transaction-less access hides a potential bug or not.
On top of that, the GRDB 2.0 API makes it painful to both run a transaction and extract values, leading to more opportunities to hesitate between transactions and convenience:
Since GRDB is supposed to take care of concurrency difficulties so that developers can profit from the safest behavior without much thinking about it, something has to be done.
After this PR is merged:
dbQueue.write
anddbPool.write
run inside a transaction.dbQueue.inTransaction
anddbPool.writeInTransaction
are still here, in order to support the various SQLite transaction types, and explicit rollbacks.dbQueue.inDatabase
, which does not open a transaction, is now documented as an advanced method.dbPool.writeWithoutTransaction
is introduced, for the advanced use cases that need fine-grained transactions.