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.

Reply via email to