[Proposal] Optimistic locking without `lock_version`

rubyonrails-core
,
1

There are some use cases where optimistic locking is useful, but doesn’t need the full complexity of a lock_version increment. For example, say a record in the database needs to be “claimed” by the first user who claims it.

| id | user_id |
|----|---------|
|  1 |  NULL   |

If I have 2 requests in parallel,

  • Request 1: Record.find(1).update(user_id: 111)
  • Request 2: Record.find(1).update(user_id: 222)

And if both objects are instantiated while user_id is still NULL, then last one will always “win”, even if user_id was no longer null at the moment of commit.

So even if first one wrote 111, the second would change it to 222.

I could use either pessimistic or optimistic locking for this. In my case optimistic makes more sense from performance standpoint. However, lock_version seems like an overkill, since all I’m really doing is:

UPDATE records SET user_id = 222 WHERE id = 1 AND user_id is NULL

Then raising an error if affected_rows.zero?.

I could technically do this using Record.update_all, but then I’m losing all the callbacks/validations/etc.

Proposal

What if we had something like update_if or save_if function that accepted the constraints hash?

account.update_if({balance: 400}, balance: 500)

and/or

account.balance = 500
account.save_if(balance: 400)

or

account.update(balance: 500).where(balance: 400)

This would support all the callback/validation stuff rails has, maybe even accept standard save **options, but also raise ActiveRecord::StaleObjectError if original balance wasn’t 400.

The exact interface of this feature is definitely up for debate. What do ya’ll think? Or is there something obvious I’m missing that does this already, while preserving callbacks?

P.S. And the existing lock_version code could then be rewritten on top of this feature, since the behavior is largely the same.

4 Likes
2

FWIW, if folks like it, I can write this PR. Just need to see if there’s some support + agreement on the API for this.

3

I would definitely use that method. There is already a (private) method that works similarly and is used to implement both update and optimist locking:

I think it’s technically simple, though I don’t particularly like the _if part. save_where or save(_where:)? Naming is hard. I guess I also wonder whether it should preserve existing default_scope or primary/composite keys (or anything else I’m forgetting that also constrains these queries).

1 Like
4

There is already a (private) method that works similarly

Funny you mention that. :slight_smile: Locally I wrote a SaveIf concern that uses that method, and inspired this proposal.

I don’t particularly like the _if part. save_where or save(_where:) ? Naming is hard.

Completely agree, I cringe at this part too. Some of the issues with naming.

  1. It’s probably not a good idea for backwards compatibility to enhance the existing save and update with this new behavior.
  2. Originally I framed this logic in terms of update (since this proposal is only relevant to updates), and had the idea of something like: update_from(constraints, to: attributes). E.g. update_from({balance: 100}, to: {balance: 200}). Unfortunately, it seemed unfair to leave out save, because there isn’t a tech reason not to let you assign attributes first, and then specify constraints upon saving. And save_from didn’t read right.
  3. The concept of where has been taken over by Arel, so could be a bit confusing to have another where for plain hash of constraints only.

Bottom line, I agree that API/naming is the hardest part here.

I also wonder whether it should preserve existing default_scope or primary/composite keys (or anything else

My initial thought here is to be naive with added constraints, and do what lock_version does.

5

IMO, that’s too wordy. You already have the object’s initial state thanks to the Dirty module. All you need is “[save|update]_unless_stale”, which will build “where” condition based on pristine, unchanged state. Just like in your example, but instead of forcing user to write the “where” by himself - you populate “where” automagically. Also, an option to AR to toggle this behaviour globally.

1 Like
6

That’s true if you do this:

account = Account.where(balance: 400).find(id)
account.update_if({balance: 400}, balance: 500) # <- the before state is redundant

And not true if you do this:

account = Account.find(id)
account.update_if({balance: 400}, balance: 500) # <- the before state is needed

Since in the latter case you don’t know whether you loaded the proper “before state” to begin with.

Your idea simplifies the interface, but is the former possible in every imagined situation? If not, maybe the “before” state could be the default, with an ability to override it.

That said, there’s also no guarantee that you want all fields participating in a lock. So at the very least you might want to specify a list of column names to constrain by, which would require a similar API.

7

Not that I use ActiveRecord, but if you want both validation and conditional update, couldn’t you just validate! then update_all as you first suggested?

The id of the validated record has to be added to the scope for update_all for this to work, so it will look something like this:

record
  .tap(&:validate!)
  .class.where({ id: record.id, user_id: nil })
  .update_all({ user_id: 111 })

Also, IMO, lock_version is not that expensive for the majority of users.

8

If you just want validations, yes.

What about callbacks? For example, updating the updated_at.

What about having the dirty/previous state reflected in the record instance (for change-dependent callbacks to be able to function)?

9

I don’t really get the “not true” part… look, in both examples you’re doing “find”, which means that you have object’s attributes. “Dirty” helps you track down object’s pristine state - state in which it was at the moment you’ve last seen it in database. If any update to any attribute of that object have been executed between fetch from DB and save attempt - there will be difference. Using “where” with all attributes instead of one specific makes it behave more in line with optimistic locking - it, too, doesn’t care which column is changed/checked. The important part is that there is a change. The approach you’re offering is very use-case specific. What I’m talking about is much more universal and robust than checking a specific column - might as well just check if lock_version have changed - same effect.

10

Okay, here’s a clearer example. Imagine locked_update works exactly as you described. In my use case, a user must claim the record. Whoever sets user_id first, wins.

Here’s a working example:

record = Record.where(user_id: nil).find(12)
record.locked_update(user_id: 777)

The above will work every time, because I SELECT’ed by user_id: nil. It’s definitely nil in memory. Perfect.

A broken example:

record = Record.find(12)
record.locked_update(user_id: 777)

The above will SOMETIMES work, because I didn’t make sure that I SELECTed user_id: nil. So by the time find executes, user_id may have already been non-null. The lock won’t help me with my use case.

I think this latter situation can happen if you selected the record to do other things with it besides claiming, but also want to claim it while you have it.

What I’m talking about is much more universal and robust than checking a specific column - might as well just check if lock_version have changed - same effect.

We need to be able to specify a column, because imagine this record has a counter cache of associated records. So each time associated records are added/removed, the counter cache update would result in stale object. Stuff like that seems unnecessary.

11

Very detailed discussions here; I don’t follow everything anymore.
As someone who doesn’t use ActiveRecord, I can’t really say anything.

I don’t oppose the conditional update idea but make ActiveRecord more bloat than it already is. Anyway, if both maintainers and users are happy, who am I to say? However, the burden lies to the maintainers, and for now, core devs still remain silent; you would have more luck rolling your own gems than the PR.

IMO, the whole ActiveRecord things are too complex. People trying to fix lots of issues originated from coupling the business logic to the ActiveRecord in the first place. These whole callbacks and counter cache issues won’t happen if the business logic is agnostic to the ORM / persistent layers.
(There might be some other way, but I did it CQRS style where the write side is pluggable to storage-agnostic business logic.)

TL;DR: I once regrets using lots of ActiveRecord’s features; other people might experience similar things too, depending on projects they are encountering. IMO: less is more for the ActiveRecord; there are plenty of features already.

If you are like me, it is probably the time to move on; you grew out of the Rails way. And it is not a bad thing; it is a sign that we are improving and made progress.

12

This is a Rails forum. :slight_smile:

13

Hey, let me elaborate.

Rails is totally fine without ActiveRecord; you can rails new without ActiveRecord.
The option --skip-active-record has been there for a very long time. And people, including myself, are actually using that.

This forum is for Rails, the library.
However, some people take Rails not just as a library but as guidance, the "Rails way"™.

The “Rails way” is only suitable for a very specific type of problem. If I were to summarise to a single sentence in my words: it is suitable for “content-centric applications”.
e.g., Blog, Email, Webboard, etc.

Not all applications focus on content.
e.g., Coupon redemption, where there are tons of conditions surrounding the business logic, Financial services, Resource reservations, etc.

If you are starting to use AR’s callbacks, there is a high chance that you are on a project that is not suitable for the Rails way, but not always; if that is the case, you can still do it that way at your expense, though.

Just be aware that Rails, the library, is much more than the Rails way; people actually use it to build things that are not content-centric.

Take this topic as an example, the project that you are working on that leads to this discussion is not focusing on the content, isn’t it?
:cowboy_hat_face:

P.S. Even if I don’t use ActiveRecord, I do use ActiveModel and everything else from Rails.

14

I personally agree with you, and use AR and callbacks in a limited way described in this blog post. It’s just that if I’m proposing this kind of feature, I want it to work well with everything else in AR, regardless of my own opinion.

15

Why not just make it a gem and see how it rolls out? If there’s a demand, you’ll have a much better indication, than a bunch of comments on a Rails forum :wink: Not every single bell and whistle have to necessarily be a part of Rails core.

16

I’m going to follow the hacker news rule, and assume good faith.

Some features are either too situational for a gem, or intertwined too much with the framework internals to be easily maintained externally. They’d still make great optional framework behavior. I find this be one of them.

Yes.

17

Could we use method chaining? E.g. record.if(…).update(…) where if could be aliased as provided or when.