 |
From: Michael Koziarski <mich...@koziarski.com>
Date: Wed, 17 Oct 2012 10:22:10 +1300
Local: Tues, Oct 16 2012 5:22 pm
Subject: Re: [Rails-core] destroy callbacks race condition
I don't think it's reasonable to force pessimistic locks on every single destroy call, my point is more that in your case it's a work around. -- Koz
On Wednesday, 17 October 2012 at 7:06 AM, Brian Durand wrote:
> I think adding a pessimistic lock to the destroy method will work. I opened this pull request that locks the record in the database before destroying it. If the record no longer exists, the callbacks are not called. > https://github.com/rails/rails/pull/7965
> This won't work on databases that don't support row locking, but you're using such a database you'd likely have other issues in a high concurrency situation like is needed to produce this issue.
> On Saturday, October 13, 2012 10:47:15 AM UTC-7, Brian Durand wrote:
> > https://gist.github.com/3885509
> > On Friday, October 12, 2012 1:36:22 PM UTC-7, richard schneeman wrote:
> > > -- > > > @schneems (http://twitter.com/schneems)
> > > On Friday, October 12, 2012 at 7:53 AM, Brian Durand wrote:
> > > > I've been looking into consistency problem with association counter caches where the counter cache value in the database is not consistent with the actual number of records in the association. What I've found is that it is from a concurrency issue where two process try to destroy the same record at the same time. Here is the pseudo SQL that is sent to the database when two process are deleting at the same time:
> > > > process_1 -> SELECT * FROM table WHERE id = 1
> > > > What happens is process_1 updates the counter cache and deletes the record. Process_2 simply updates the counter cache because the record is already deleted by the time it tries to delete it.
> > > > This is a pretty complicated issue and it touches more than just this one test case. The problem being that all the before and after destroy callbacks will be called regardless of if the record is actually destroyed. In the particular case of the counter caches, I think it could be fixed by moving the callback from a before_destroy to an after_destroy and adding a check in ActiveRecord to only call after destroy callbacks if a row was actually removed from the table.
> > > > In general I think it would be correct to make this general behavior so that after_destroy callbacks are not called if no record was deleted. However, that could affect quite a few things inside application code which could potentially leave objects in an inconsistent state because an expected callback was not called. I think the pending upgrade to Rails 4.0 might be a good time to introduce such behavior since it's a major upgrade and as such people should not be expecting applications to work 100% without some alterations. This does not touch on the issue of before_destroy callbacks which would not be able to check the status of the delete operation. This could be handled with documentation stating that this is a known issue.
> > > > Another solution that would have less effect on current applications (but also leave them more vulnerable to being in an inconsistent state) would be to provide some sort of flag within the record that after_destroy callbacks could check if they are persisting data or interacting with external systems. Something like "row_deleted?" so that callbacks could be defined as:
> > > > after_destroy :my_callback, :if => :row_deleted?
> > > > Thoughts? > > > > -- > -- You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
| |||||||||||||