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: > > I've put a stand alone script here that reproduces the issue: > > https://gist.github.com/3885509 > > > On Friday, October 12, 2012 1:36:22 PM UTC-7, richard schneeman wrote: >> >> Can you write a public rails app that reproduces this issue? This >> behavior would be undesired and therefore a bug. If we can reproduce and >> attach that to an issue it could help the discussion. >> >> -- >> Richard Schneeman >> http://heroku.com >> @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 >> process_2 -> SELECT * FROM table WHERE id = 1 >> process_1 -> BEGIN >> process_2 -> BEGIN >> process_1 -> UPDATE parent_table SET counter_cache = >> COALESCE(counter_cache, 0) - 1 WHERE id = 1 >> process_1 -> DELETE FROM table WHERE id = 1 >> process_1 -> COMMIT >> process_2 -> UPDATE parent_table SET counter_cache = >> COALESCE(counter_cache, 0) - 1 WHERE id = 1 >> process_2 -> DELETE FROM table WHERE id = 1 >> process_2 -> COMMIT >> >> 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 received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To view this discussion on the web visit >> https://groups.google.com/d/msg/rubyonrails-core/-/KnPOlQzxj2cJ. >> To post to this group, send email to [email protected]. >> To unsubscribe from this group, send email to >> [email protected]. >> For more options, visit this group at >> http://groups.google.com/group/rubyonrails-core?hl=en. >> >> >> -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/MyWn00d6O-MJ. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
