Data Inconsistency on Real Time Index caused by after_save

[slatem]

I'm experiencing some data inconsistency in production that is difficult to replicate in testing.

In the past I've been bitten by bugs related to using after_save and I'm thinking that perhaps the same thing is happening here. To remedy, I've changed:

after_save ThinkingSphinx::RealTime.callback_for(:model_name)

to

after_commit :sphinx_update

def sphinx_update
		ThinkingSphinx::RealTime.callback_for(:model_name).after_save self
end

Any drawbacks by doing this that I'm missing? Is this a real issue or something else I'm missing?

[pat]

I'm curious as to the specific cases when you've lost data changes - if you know the finer details? - but your after_commit approach should work fine. It's something I'll ponder further - maybe it's worth changing TS's behaviour to match, or at least allowing it to be configured.

The one thing you'll lose out on this way, though, is that using after_save means that you can use transactional fixtures in your tests. I'm not sure if after_commit is invoked at all with that setup, so Sphinx results may not be accurate when running tests.

[slatem]

I found the problem in my particular case. I was updating an object from another object (bad code smell, I know, it's a transitional use case), and in doing so the callback obviously didn't realize the model had changed. I performed a model.reload prior to the model.save and everything worked as expected.

I have been bitten by the after_commit vs after_save before though so I'm inclined to leave it this way.

With regards to tests, you could always:

after_save :sphinx_update, :if=>Rails.env.test?

Not pretty but you could turn it into a module and include all the code that way.

[pat]

Just made the callback object work with both after_save and after_commit:

# both of these will work - but only use one!
after_save ThinkingSphinx::RealTime.callback_for(:model_name)
# or
after_commit ThinkingSphinx::RealTime.callback_for(:model_name)

[jdelStrother]

I'm curious as to the specific cases when you've lost data changes

FWIW I think we're reproducing this due to something like the following (pseudocode-ish):

class Post < ActiveRecord::Base
  has_one :image
  after_save ThinkingSphinx::RealTime.callback_for(:post)
end

class Image < ActiveRecord::Base
  belongs_to :post
  validate :valid_image_attributes?
  def valid_image_attributes?
     errors.add(:base, "Oh no something went wrong")
  end
end

If you create the post & its image in one go, then:

• the post will be inserted to mysql (or your favorite database)
• ThinkingSphinx's after_save will kick in
• the image will be validated. It's not valid, so triggers a rollback in mysql.

and you're then left with a record in sphinx and no corresponding record in mysql.

Think it's worth ThinkingSphinx handling the after_rollback somehow? Or should we just switch to after_commit everywhere?

[pat]

Thanks for that use-case Jonathan, that's super useful :)

I've just pushed a commit to the develop branch which allows using the standard TS deletion callback via after_rollback, so you can now put the following into your model alongside the existing after_save:

after_rollback ThinkingSphinx::ActiveRecord::Callbacks::DeleteCallbacks,
  :on => :create

Still pondering how far I want to automate this, and how often that it could be a regular situation. Feedback's certainly welcome :)