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

Ensure update_all series cares about optimistic locking #35352

Merged

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Feb 21, 2019

Implicitly updating the lock version without knowing the current version
is meaningless.

@matthewd
Copy link
Member

Incrementing the lock version invalidates any other process's optimistic lock, which is the desired outcome: the record no longer looks the same as it did when they loaded it.

@kamipo
Copy link
Member Author

kamipo commented Feb 21, 2019

If so, we need the update not only in touch_all but also in update_counters and update_all.

@kamipo
Copy link
Member Author

kamipo commented Feb 21, 2019

Changed to opposite direction c14e161.

Incrementing the lock version invalidates any other process's optimistic
lock, which is the desired outcome: the record no longer looks the same
as it did when they loaded it.
@kamipo kamipo force-pushed the update_all_doesnt_care_optimistic_locking branch from c14e161 to 1d8fad6 Compare February 25, 2019 11:16
@kamipo kamipo merged commit 66d40ab into rails:master Feb 25, 2019
@kamipo kamipo deleted the update_all_doesnt_care_optimistic_locking branch February 25, 2019 11:49
@kamipo kamipo changed the title Ensure update_all series doesn't care optimistic locking Ensure update_all series cares about optimistic locking Feb 25, 2019
@kamipo kamipo mentioned this pull request Mar 7, 2019
suketa added a commit to suketa/rails_sandbox that referenced this pull request Aug 18, 2019
Ensure `update_all` series cares about optimistic locking
rails/rails#35352)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants