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

How does Waterline handle update concurrency ? #5328

Closed
girishla opened this issue Dec 23, 2015 · 10 comments
Closed

How does Waterline handle update concurrency ? #5328

girishla opened this issue Dec 23, 2015 · 10 comments

Comments

@girishla
Copy link

Apologies if this is just a silly noob question.
I was wondering how Waterline handles concurrent updates to the same record by different users.

like so...

  1. User A executes findone(id=1)
  2. User B executes findone(id=1)
  3. User A and B are both simultaneously making changes to the exact same record albeit different changes.
  4. User A executes save()
  5. User B also executes save()

Depending on which save() is committed to the DB first, one user will lose their changes because it will be overwritten by the second one.

Is there support for handling this scenario in Waterline ?

@devinivy
Copy link

Waterline does not have transaction support, though there's an issue for that here: https://github.com/balderdashy/waterline/issues/755 . This also relates to @RWOverdijk's dirty checking PR balderdashy/waterline#1048, which I pray gets revisited some day. With that PR the changes made by save() would be the minimum necessary changes.

@girishla
Copy link
Author

hmm...
Transaction support would only help concurrency control within a single request/commit cycle. For ones spanning multiple cycles involving "user think time", we would need a some sort of locking mechanism.
This would fall under the "Locking" concurrency bucket.

The most common pattern Ive seen to solve this is Optimistic Locking where a version number is used to check if data has been updated by another request.

@devinivy
Copy link

Ah yes, I see what you're saying. If you have an idea how this would be implemented in waterline's cross-adapter environment, I'm sure we'd be open to seeing a spec!

@randallmeeker
Copy link

All my models have a beforeUpdate that checks the udpatedAt field on the record to confirm the record has not been updated previously updated. It works the same as a validation test. If the validations fails, the update is not performed and user is notified that the record their viewing is out of date.

@girishla
Copy link
Author

@randallmeeker but that would involve another round-trip to the datastore ?

It's fairly straight-fwd to solve this for the relational world where you would have a composite Unique Constraint (PK, VERSION_NUM) on all Tables. All ORM models would read the VERSION_NUM as part of all queries and all models would do a VERSION_NUM +1 on update calls. If the Unique Constraint is violated, the constraint violation error is bubbled up all the way to the user and reads "Sorry, someone else has updated the record since you last retrieved it".
This is a fairly ugly user error but should help preserve data integrity and robustness.

Now, I'm only a fews days old in the Waterline world so am in no position to implement this in a cross-adapter fashion. Hopefully in some time after I get to grips with the code, I can contribute.

In the meantime, do you reckon this stands a chance of being implemented as it is vital for me if I am to select this ORM for enterprise scale implementation ?

@devinivy
Copy link

It does stand a chance, but it's worth keeping in mind that there's a pretty long roadmap, and currently waterline is in "tighten all existing features" mode.

I could be wrong, but this sounds like something you can implement somewhat easily using lifecycle callbacks and manually creating the unique constraint– especially with the pending PR balderdashy/waterline#1122 which allows you to mutate update criteria. Non-sequel databases will probably need different strategies.

@girishla
Copy link
Author

Yes, you're right @devinivy I should be able to implement this manually as I am still at the POC stage.
I would appreciate it if gets onto the roadmap so I can point to it when questioned.

I think Waterline does a fantastic job of acting as a cross-adapter facade. I guess we'd all be living in a perfect world if it also supports some of these raw services such as transactions, locking etc :)

Thanks for your help. Much appreciated.

@devinivy
Copy link

No problem! I agree– this would be a great feature. I invite you to make a pull request to the ROADMAP.md file!

@kevinburkeshyp
Copy link

yeah... this is a fun problem. we generally avoid .save() in all of our code that has a chance of concurrent access, and instead issue UPDATE statements with a where clause - see https://kev.inburke.com/kevin/faster-correct-database-queries/ for more info.

@nrempel
Copy link

nrempel commented May 26, 2016

I thought I would drop this here for visibility.

If you need to use distributed locking in your application, this library is very handy: https://github.com/blockai/advisory-lock

@raqem raqem transferred this issue from balderdashy/waterline May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants