On Saturday, 13 October 2012 at 6: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. Generally speaking that sounds like a sane change, but definitely something that we should experiment with a *lot* before making the changes.
As for your particular issue, the simplest way to avoid them is to fetch the records with :lock=>true so the database does a SELECT … FOR UPDATE, relational databases are pretty good at preventing this kind of thing, best use them if you can. > > 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] > (mailto:[email protected]). > To unsubscribe from this group, send email to > [email protected] > (mailto:[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 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.
