karmi
Tire does not respect ROLLBACK of ActiveRecord record
When I save a record which is then rolled back, the ElasticSearch database still has the outdated entry.
#343 talks also about the after_commit and after_rollback callbacks, but they're not yet supported.
https://github.com/karmi/tire/blob/master/lib/tire/model/callbacks.rb
Hi, absolutely right. The after_commit / after_rollback hooks are ActiveRecord specific, so we must take care here. Since the callback integration is very simple, I think we can add it generally, not with a opt-in layer. I will have a look into it this week, please ping me if I slip.
@tmaier Look at #640, actually, using after_commit would introduce inconsistencies between the database and the search index. This will need some more thorough thinking -- any comments, ideas?
What about if we leave after_save and add a after_rollback callback, which destroys the ES entry?
This would work with new records, but would create new inconsistencies when updating an existing record. Unless we cache the old entry...
@karmi you asked me to remind you. :)
At the moment, my database also has inconsistencies, as there are records in the ES database which got rolled back (for some reason) by the Postgre Database.
Let me add to this that having the reindexing done on after save can also lead to severe problems if the elasticsearch server cannot be reached. In our case this was happening because it could not connect to the IP address of the elasticsearch server and would just sit there and timeout. Since we were still in a transaction the whole mysql update would fail with a Lock timeout error, effectively rendering the elasticsearch server into a single point of failure.
@donaldpiret True! But the way I see it, if you're concerned about the HTTP call inside the update_index on each save, you should have a specific indexing strategy, ie. backgrounding the index operation (Threads, Resque, Sidekiq, RabbitMQ, etc). (No argument though, that for ActiveRecord::Base models, we should fire indexing on commit, not save.)
I ran into this yesterday, when a create and update_attributes got rolled back. So the entry in the database was deleted and the ElasticSearch document remained.
I was thinking the 'after_callback' would have to check the database entry and update/delete ElasticSearch accordingly.
@jeremygpeterson See the caveat in 38492a5832160642b669630892554c74c7101f5c. I'd like to have a look into the hooks in Tire to make use of the after_commit.
@karmi I see the caveat in the readme now, I didn't realize that including Tire::Model:Callbacks did not account for all possibilities on the active record model.
How can I help on this issue, keeping the database synced with ElasticSearch really matters to me. I'm not sure if <after_commit> would handle my specific error. As the rollback occurs during the creation process, but I'm open to implementing something for you to look at.
This is how I handled the transaction when a create is rolled back.
if base.respond_to?(:after_rollback)
index_delete = lambda {
@destroyed = true
tire.update_index
}
base.send :after_rollback, index_delete, on: :create
end