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

Remove support for nested transactions #711

Closed
dstillman opened this issue May 9, 2015 · 0 comments
Closed

Remove support for nested transactions #711

dstillman opened this issue May 9, 2015 · 0 comments
Assignees
Milestone

Comments

@dstillman
Copy link
Member

Traditionally, our DB layer has supported automatically nested DB transactions. If you called Zotero.DB.beginTransaction() twice, you ended up in transaction level 1 (up from 0), and commitTransaction() had to be called twice before the changes were actually committed. This allowed data layer functions to wrap SQL statements in transactions and be sure that the data would remain consistent, regardless of whether the function was called directly or called by another function that had already opened a transaction itself.

With async DB access, I don't think we can safely support nested transactions. Since statements within async transactions yield, a second, unrelated executeTransaction() call could come in in the middle of a transaction. At best, this would mean that the unrelated transaction would automatically nest, such that a failure of one would affect the other. At worst, this could result in statements from the second transaction being run outside a transaction, since the first transaction could complete first and be closed in Sqlite.jsm.

My solution is to remove support for nested transactions and have all transactions block (up to a timeout) on other transactions. Unfortunately, this means that functions can't open transactions if there's any chance that they could be called (at any point up the stack) by other functions that open transactions, since the called function will block on the calling function. And since those called functions still require data consistency, they need to fail if called outside a transaction (Zotero.DB.requireTransaction()).

Having to know the transaction requirements of functions from above is definitely a pain, but I don't have a better solution. In most cases, at least, I think it will be clear where the transaction should go.

Zotero.DataObject.prototype.save() is a special case, since it's called so frequently on its own (e.g., in various UI code, all throughout tests, in third-party code), so I'm creating a saveTx() function that tells save() to start a transaction itself. Any code that just creates an individual objects can use that. I think I'm also going to have save() start a transaction automatically (which in most cases won't be a problem) if one isn't open but log a warning advising the use of saveTx(), just so that everything doesn't break while we're fixing old calls.

@dstillman dstillman self-assigned this May 9, 2015
@dstillman dstillman added this to the 5.0 milestone May 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant