after_destroy callbacks sometimes not called with execute_after_commit and transaction

[marinfr]

Hello 👋

I've recently started using this gem's execute_after_commit feature to successfully fix MySQL LockWaitTimeout issues, but now I've stumbled upon a nasty bug. In some cases, the counter is not updated after destroy with execute_after_commit.

Let's say we have:

class Graph < ApplicationRecord
  has_many :graph_nodes, class_name: 'Node', primary_key: 'root_id'
end

class Node < ApplicationRecord
  belongs_to :graph,  optional: true, foreign_key: 'graph_id', primary_key: 'root_id'
  counter_culture :graph, execute_after_commit: true # column is 'nodes_count'
end

And then we have a transaction where an update before destroy is happening:

ActiveRecord::Base.transaction do
  Node.last.touch
  Node.last.destroy
end
# => no decrement query

ActiveRecord::Base.transaction do
  Node.last.touch
  Node.last.destroy
  Node.first.destroy
end
# => only the last destroy will trigger one decrement query

Works well without execute_after_commit: true or by changing the code (not possible in my case) to:

ActiveRecord::Base.transaction do
  node = Node.last
  node.touch
  node.destroy
end

I suspect the update request somehow prevents the after_destroy callback.

[magnusvk]

Hey @marinfr thanks for reporting this. I did some digging, see #316. However, my take-away from that work is that the after_commit hook simply is not called at all for your issue here. So at first glance, this looks like more of a Rails problem than a counter_culture issue. Can you take a look at the output in #316 and let me know what you think?

[marinfr]

@magnusvk, thanks for looking into this!

I've tried to reproduce this behaviour with basic Rails by defining an after_commit action that just prints AFTER_COMMIT. It's for a different model that doesn't have anything to do with counter columns. Trying both syntaxes of updating+destroying, AFTER_COMMIT was always and only printed once at the end:

   (2.2ms)  COMMIT
"AFTER COMMIT"

Does this mean that after_commit not being called is not the issue here? It looks like it's the execute_after_commit logic in this or the after_commit_action gems.

[marinfr]

Ok, you might be right!

I've defined this in the model described in the issue:

after_commit :test_after_commit

def test_after_commit
  actions = {}

  actions[:create]  = transaction_include_any_action?([:create])  || nil
  actions[:update]  = transaction_include_any_action?([:update])  || nil
  actions[:destroy] = transaction_include_any_action?([:destroy]) || nil

  p "AFTER COMMIT EXECUTED FOR #{actions.compact.keys.join(", ")}"
end

The output with the first approach:

   (2.4ms)  COMMIT
"AFTER COMMIT EXECUTED FOR update"

The output with the second approach:

   (2.0ms)  COMMIT
"AFTER COMMIT EXECUTED FOR destroy"

Feel free to close this issue as it looks like it's intended for the Rails team.
Thanks!

[magnusvk]

Bummer, I personally don’t think that’s the right behavior, but also wouldn’t know what to do to fix it!