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

Support for optimistic locking #16

Closed
benjcooley opened this issue Aug 18, 2015 · 10 comments
Closed

Support for optimistic locking #16

benjcooley opened this issue Aug 18, 2015 · 10 comments

Comments

@benjcooley
Copy link

.NET and Java DynamoDB sdk support optimistic locking using conditional constrainst and a special annotated numeric 'version' field in the schema.

This could be implemented in dynamoose via uuid and a tagged 'version' field in the schema.

_version: {
  version: true,  // This indicates this field is the version number field
  type: Number
  required: true
}

Whenever a record is updated and the '_version' field is passed, dynamoose will automatically add a conditional constraint to ensure the write only succeeds if the _version field matches.

Would be happy to implement and send a PR. Also would be happy to make this a plugin if there were a plugin interface.

@brandongoode
Copy link
Contributor

A PR would be great. The feature will be very useful. Thanks.

@dolsem
Copy link
Contributor

dolsem commented Nov 13, 2018

I would like to help with this. Suppose we add a boolean schema option optimisticLocking and when it's true we maintain a field called '_version'. Should this field be exposed to the user? If yes, should it just be a slightly special attribute like timestamps, which the user can manipulate and write custom definition for? Personally I think we should probably just hide it completely. Can't think of any reason to expose it.

@fishcharlie
Copy link
Member

@dolsem I see the purpose of Dynamoose to be to make it easier to use DynamoDB and functionality like this. I don't see any reason to expose complexity unless there is a good reason for it. That being said, it might be beneficial for advanced or strange use cases to optionally expose it. But only if the developer wants to take advantage of it. Not sure if that would be more complicated to build in tho.

I see the value of making this part of Dynamoose itself, and I could be wrong, but I think there was some discussion about writing a Dynamoose plugin for this, and not implementing it into the library itself. #325

@dolsem
Copy link
Contributor

dolsem commented Nov 14, 2018

@fishcharlie well I can do either, so let me know what makes most sense to you. Personally I view it as a core feature, because databases are often accessed concurrently, and there needs to be a way to ensure concurrent writes work correctly. However, if you think it's best to make it into a plugin, I can do that, no problem.

@fishcharlie
Copy link
Member

@dolsem Part of me wants to say that we need testing of that plugins PR. BUT I think integrating it directly into Dynamoose is the correct path to take in my opinion.

@dolsem
Copy link
Contributor

dolsem commented Nov 14, 2018

@fishcharlie I'm a bit confused: by direct integration you mean not choosing the plugin path, right? Then how's testing of the plugins PR relevant to this context? Or are you saying, ideally it should be a plugin, but only given the plugins PR has no issues and can be merged?

@fishcharlie
Copy link
Member

@dolsem Sorry for the confusion. It should be implemented directly into this repository and codebase. Not as a plugin.

The only benefit I see to going down the plugins route is that it would give us more testing of that system. Which isn't enough of a benefit to go down that path in my opinion.

Feel free to work on this and submit a PR if you want 😃

@dolsem
Copy link
Contributor

dolsem commented Nov 14, 2018

Okay, got it.

@fishcharlie
Copy link
Member

@dolsem Actually I changed my mind on this. Unless someone has a strong case for being in Dynamoose directly, I think a plugin would be best suited for this task.

I'm closing this issue and moving it to the plugin ideas repo.

dynamoose/dynamoose-plugin-ideas#3

@ramharsha-pe
Copy link

@fishcharlie In cases of concurrency where multiple servers have to update a record and atomic updates are not an option Optimistic Lock Control feature can be of great help.

for eg. lets say I have to update the average cancelled orders of a product id every time somebody cancels an order with a productid, we will have multiple servers performing the same calculation and end up with botched records.

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

No branches or pull requests

5 participants