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

Optimistic Locking #100

Open
3 tasks
Fs02 opened this issue Sep 18, 2020 · 4 comments
Open
3 tasks

Optimistic Locking #100

Fs02 opened this issue Sep 18, 2020 · 4 comments
Labels
hacktoberfest help wanted Extra attention is needed

Comments

@Fs02
Copy link
Member

Fs02 commented Sep 18, 2020

Background

Optimistic locking (or optimistic concurrency control) is a technique that allows concurrent edits on a single record (more: https://hexdocs.pm/ecto/0.9.0/Ecto.Model.OptimisticLock.html)

Implementation

  • Detect Optimistic Locking enabled when a struct contains a LockVersion int field.
  • Use LockVersion field whenever Update or Delete is performed to prevent operation on stale version of record. Maybe we can simply add additional where filter LockVersion=? on update or delete query.
  • Differentiate error when updating/deleting stale record for non existent record.
@Fs02 Fs02 created this issue from a note in Development v1 (To do) Sep 18, 2020
@Fs02 Fs02 added the help wanted Extra attention is needed label Sep 18, 2020
@dranikpg
Copy link
Contributor

dranikpg commented Feb 9, 2022

There are at least two common approaches to optimistic locking I know:

  • using a where version = ? clause
  • using a returning version and checking versions in code (this works only in transactions, as we have to abort without changes)

The first approach is more flexible (can be used in any context), however it doesn't allow us to differentiate between errors for stale and nonexistent records. Is this that big of an issue? If we already have a record with a version, we know it existed (at least in the past). The user would have to get more info about the confict manually on abortion (like querying record with version again)

What about turning locking off? Should it require special functions for saving/deleting? Given, that embedded structs can be supported, enabling/disabling could be done with wrapper types.

@Fs02
Copy link
Member Author

Fs02 commented Feb 10, 2022

The first approach is more flexible (can be used in any context), however it doesn't allow us to differentiate between errors for stale and nonexistent records. Is this that big of an issue? If we already have a record with a version, we know it existed (at least in the past). The user would have to get more info about the confict manually on abortion (like querying record with version again)

yes I agree, the size of issue is depends, but I think it's not possible to differentiate stale and nonexistent records, I think we just need to state this explicitly in the documentation

What about turning locking off?

if user want to update/delete while ignoring lock, user should be able to use rel.Unscoped()
example: repo.Update(ctx, &book, rel.Unscoped()) / repo.Delete(ctx, &book, rel.Unscoped())

@dranikpg
Copy link
Contributor

Having repo.Delete(ctx, &model, rel.Unscoped()) is not possible with the current signature, which accepts only a bool vararg for cascades. It it possible to change Delete() to accept mutates or must strong backward compatibility be maintained?

@Fs02
Copy link
Member Author

Fs02 commented Feb 18, 2022

It it possible to change Delete() to accept mutates or must strong backward compatibility be maintained?

It's okay, this shouldn't break the compilation, and REL is still in unstable version (0.x.x) is due to this possible API change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest help wanted Extra attention is needed
Projects
Development v1
  
To do
Development

No branches or pull requests

2 participants