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

Transactions #163

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Transactions #163

wants to merge 4 commits into from

Conversation

alecgibson
Copy link
Contributor

No description provided.

// we have two different transactions attempting to update the same collection.
// Here, we manually keep track of the collections involved in transactions, so that
// we can avoid this deadlock.
var isLocked = this._lockedCollections[collectionName] &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in share/sharedb#689 (comment) this locking complexity may be removed entirely if we follow MongoDB's own behaviour and define cross-transactional, awaited writes to the same document to be undefined behaviour. Instead, if we just let them run in parallel, MongoDB and ShareDB can do their thing.

// Here, we manually keep track of the collections involved in transactions, so that
// we can avoid this deadlock.
var isLocked = this._lockedCollections[collectionName] &&
this._lockedCollections[collectionName] !== options.transaction;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: if we keep this behaviour (I don't think we should), I think MongoDB locks on a document level, so we should update this key to collectionName + id to avoid unnecessary locking.

// current transaction
// TODO: Wait for this?
this.abortTransaction(this._lockedCollections[collectionName], function(error) {
// TODO: Handle errors
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure what to do with this error, and this feels like a bit of a code smell in the architecture of transactions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant