Need advice about race condition

Rails
1

There’s a field in my User model that sometimes needs bulk updates:

User.update_all(“cluster = #{new_cluster}”, “cluster = #
{current_cluster}”)

Now, suppose user with id 37 is affected by that particular
update_all and updates his profile:

  1. user 37 is read
  2. update_all happens and changes 37’s cluster asynchronously
  3. user 37 is saved

now user 37 belongs to the wrong cluster. Unlikely but possible. Is
there a robust way to write that?

– fxn

2

Xavier N. wrote:

  1. user 37 is saved

now user 37 belongs to the wrong cluster. Unlikely but possible. Is
there a robust way to write that?

You might want to read:

zsombor

–
Company - http://primalgrasp.com
Thoughts - http://deezsombor.blogspot.com

3

Does user 37 have the ability to change his own cluster? If not, then
you’ll
only update the fields that your form submits… not everything.
(provided
you use update_attributes to save the results)

If he can update his cluster, then the only thing that comes to mind
is to
look at the modified date/time of the record you have out and compare it
to
what’s in the database.

Add a field to your table called updated_at

then in your User model, do a callback

before_update :check_if_stale

protected
def check_if_stale
user_updated_at = User.find(self.id).updated_at
if user_updated_at == self.updated_at
#record is ok to update
else
# your copy is stale… better do some corrective stuff…
could
return false which would prevent the record from being saved.
end
end

Not sure if that’s the best approach… someone else could have a
cleaner
way of doing that.

4

On Sep 1, 2006, at 1:51 PM, Brian H. wrote:

Does user 37 have the ability to change his own cluster? If not,
then you’ll only update the fields that your form submits… not
everything. (provided you use update_attributes to save the results)

Yeah, I don’t want to force all the system to use update_attributes
though, sounds brittle. I prefer a solution that still lets people
use AR models normally (except for non-bang-saves, who can raise now
an exception as save! does).

– fxn

5

On Sep 1, 2006, at 2:23 PM, Xavier N. wrote:

On Sep 1, 2006, at 1:41 PM, Dee Z. wrote:

You might want to read:
Optimistic concurrency control - Wikipedia

Oh yes, I had forgotten lock_version. Thank you!

Too quick, since that field is changed by an update_all lock_version
is untouched. I could do a select+assign in a before_save but there’s
still room for a race condition there, hmmmmm.

– fxn

6

@Xavier:

Did you try my solution that used the updated_at column in the
before_update?

7

On Sep 1, 2006, at 1:41 PM, Dee Z. wrote:

You might want to read:
Optimistic concurrency control - Wikipedia

Oh yes, I had forgotten lock_version. Thank you!

– fxn

8

On Sep 1, 2006, at 11:31 PM, Brian H. wrote:

@Xavier:

Did you try my solution that used the updated_at column in the
before_update?

That solution has another race condition, the fatal sequence within
before_update is:

  1. updated_at is read and coincides, fine
  2. update_all happens in a separate process
  3. save --> wrong

which is the same sequence as in the more straightforward

  1. cluster is read and coincides, fine
  2. update_all happens in a separate process
  3. save --> wrong

Perhaps that approach plus a mandatory transaction in all user
updates will be the only robust solution.

– fxn

9

On Sep 2, 2006, at 12:02 AM, Xavier N. wrote:

which is the same sequence as in the more straightforward

  1. cluster is read and coincides, fine
  2. update_all happens in a separate process
  3. save → wrong

Perhaps that approach plus a mandatory transaction in all user
updates will be the only robust solution.

I was kind of assuming callbacks did not run in a transaction, but
they actually do according to

Peak Obsession
ClassMethods.html

so I think I am pretty close to getting this right, great!

– fxn

10

On 9/1/06, Xavier N. [email protected] wrote:

updates will be the only robust solution.

I was kind of assuming callbacks did not run in a transaction, but
they actually do according to

Peak Obsession
ClassMethods.html

so I think I am pretty close to getting this right, great!

Your original update_all is sufficient if you include
lock_version=lock_version+1. Then dirty readers will get a
StaleObjectError
if they save after the cluster change.

jeremy