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

Save on condition (Optimistic concurrency) #4004

Closed
Fonger opened this issue Mar 20, 2016 · 18 comments
Closed

Save on condition (Optimistic concurrency) #4004

Fonger opened this issue Mar 20, 2016 · 18 comments
Labels
new feature This change adds new functionality, like a new method or class plugin
Milestone

Comments

@Fonger
Copy link
Contributor

Fonger commented Mar 20, 2016

Background

This is a proposal for optimistic concurrency for mongoose.

We sometimes use the following code to modify the document instead of update because validation is not ok (runValidator doesn't support everything)

let doc = yield Model.findById(id);
doc.prop = 123;
doc.val += 5;
yield doc.save();

However this is troublesome when another request is also modifying this document, it may cause the doc overwritten without any errors. I don't know what to call this phenomenon, maybe race condition

Optimistic Concurrency Control

A better solution is that, when reading the model, store some important values, e.g. updatedAt or __v version key.

and when updating, add a condition that the timestamp should be the same.
This mechanism is called Optimistic concurrency control, or OCC or update-if-current.

Reference: Mongodb Official document: Update Document if Current

let doc = yield Model.findById(id);
let timestamp = doc.updatedAt;

let updateProps = {
  prop: 123,
  val: doc.val + 5
};

// update only when timestamp match the original one
let updater = yield Model.update({ _id: doc._id, updatedAt: timestamp }, { $set: updateProps });

if (updater.result.nMatched === 0) {
   // if update operation doesn't match, it means this document is modified by others or is removed.
   throw new OCCError('This document is modified by others at this moment. Please try again');
}

//successful!

However, this is very ugly.
Hence I propose an elegant way in mongoose.

Proposal 1 (General)

let doc = yield Model.findById(id);
doc.prop = 123;
doc.val += 5;
yield doc.save({cond: {updatedAt: doc.updatedAt }}); // if condition doesn't match, throw an error.

Proposal 2 (Native Support)

The previous proposal is a general way.
Maybe we can include occ during find:

let doc = yield Model.findById(id).occ('updatedAt');
doc.prop = 123;
doc.val += 5;
yield doc.save(); //an OCCError is thrown if `updatedAt` doesn't match the first one 

If there are multiple values needed to be checked, use this way

let doc = yield Model.findById(id).occ('firstProp secondProp')
// or
let doc = yield Model.findById(id).occ('firstProp').occ('secondProp')
// prop in subdocument
let doc = yield Model.findById(id).occ('parent.child')

Proposal 3 (Schema options)

new Schema({..}, { saveIfCurrent: '__v' });
new Schema({..}, { saveIfCurrent: 'updatedAt parent.child' });
schema.set('saveIfCurrent', 'updatedAt parent.child');

update-if-current is the formal name.
Here, we emphasize that this behavior is working on save-if-current.

@vkarpov15
Copy link
Collaborator

How is this different from versioning, other than the fact that versioning only affects array fields?

@Fonger
Copy link
Contributor Author

Fonger commented Mar 21, 2016

@vkarpov15 Yes, it's same but versioning only affects array field.

@vkarpov15
Copy link
Collaborator

It should be pretty doable to implement this as a plugin for now. I'm hesitant to put it as a priority for core because versioning already confuses people a lot and is unnecessary in many cases. I think the way forward is plugin for now and if it turns out to be indispensable we can put it in core

@mika-fischer
Copy link

@vkarpov15 Could you give a quick pointer as to how this could be implemented as a plugin? I don't see how to hook into the .save() logic properly...

@vkarpov15
Copy link
Collaborator

Actually I'm not entirely sure if it's possible to do this with save() at the moment. There are ways to do it with update, but we'd need a way to update the query that we send to the server with save()

@vkarpov15 vkarpov15 added this to the 4.7 milestone Sep 3, 2016
@vkarpov15 vkarpov15 added the new feature This change adds new functionality, like a new method or class label Sep 3, 2016
@Fonger
Copy link
Contributor Author

Fonger commented Jan 30, 2017

@vkarpov15 Thanks. If I have some free time, I'll try to make a plugin for this.

@vkarpov15
Copy link
Collaborator

8b4870c should give you the general direction of how one would write a plugin for this

@binki
Copy link

binki commented Jun 23, 2017

Is there a suggested plugin for this?

@vkarpov15
Copy link
Collaborator

Not that I know of, but this test is the general idea of how you would implement the plugin. I would love it if someone from the community could pick it up and run with it, I don't have much experience with OCC and I feel like I wouldn't be the right person to maintain it.

@RobinJayaswal
Copy link

@vkarpov15 can you elaborate on what you mean by "versioning only affects array fields?" Its my understanding that versioning is on a document, so no matter what field you try to update it will fail if version is out of date.

@vkarpov15
Copy link
Collaborator

That's correct, __v is stored on a document. What I was trying to convey is that __v is only incremented if you modify array fields in place using, say, slice().

@lonix1
Copy link

lonix1 commented Dec 31, 2018

Has anyone had success using this plugin in production?

Coming from a .NET/Java background, where optimistic concurrency in enterprise systems is widely supported (and unavoidable), lack of support here was surprising.

I don't yet know enough about node/mongoose/mongodb so I'm scared to try some "random" plugin, so if anyone has enterprise experience with this please let us know how you tackled this problem?


Side note: why was this issue closed? Is OC supported natively without plugins?

@vkarpov15
Copy link
Collaborator

@lonix1 we added the ability to implement OCC plugins, and it looks like mongoose-update-if-current does exactly what we suggest. It looks like a solid plugin 👍 Caveat is that it only handles save(), not updateOne(), etc.

OCC hasn't really been a concern because for most apps I've worked on, Mongoose's ability to only update the paths that have actually changed is good enough to prevent accidentally overwriting.

@lonix1
Copy link

lonix1 commented Jan 5, 2019

@vkarpov15 Thank you for clarifying. I guess I'll use that plugin!

I'm surprised though that optimistic concurrency (and related concurrency patterns) isn't natively supported... I notice that sequelize has support. Mongoose is otherwise a pleasure to use, so many thanks.

@mobicity
Copy link

What is the difference between this plugin and the VersionError already built-in mongoose?

@vkarpov15
Copy link
Collaborator

@mobicity Mongoose versioning is a limited subset of OCC that only looks at array fields.

@olawalejuwonm
Copy link

Please is there any support for this yet? Seems https://www.npmjs.com/package/mongoose-update-if-current is dead as there is no recent update again

@vkarpov15
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class plugin
Projects
None yet
Development

No branches or pull requests

9 participants