meilisearch-rails icon indicating copy to clipboard operation
meilisearch-rails copied to clipboard

Allow model saving to pass even if MeiliSearch instance is offline

Open mech opened this issue 3 years ago • 8 comments

Hi, I have a use-case where I want my model to still be able to perform basic saving to database even if MeiliSearch instance is not started or found.

I don't think setting the raise_on_failure to false is the correct approach if I understand it correctly.

Specifically I am hitting this exception whenever I want to save my ActiveRecord model.

MeiliSearch::CommunicationError: An error occurred while trying to connect to the MeiliSearch instance: Failed to open TCP connection to localhost:7700 (Connection refused - connect(2) for 127.0.0.1:7700)

I understand that this is due to server not started, but I think this is quite a common situation and normal Rails persistent operation should still pass without failure even if MeiliSearch is not online. Is this possible?

mech avatar Sep 08 '22 12:09 mech

Hi @mech, I'm sorry but I couldn't investigate this issue yet.

I did not forget about it, I just need some time to make it a priority. Hope you will understand!

Take care,

brunoocasali avatar Sep 23 '22 13:09 brunoocasali

Wow no worry, everyone has their time constraint and priority. Cheer 😀 and happy weekend!

mech avatar Sep 23 '22 13:09 mech

@mech I had this same problem, the way I solved it was the following:

  after_touch do
    if Rails.env.production?
      begin
        index!
      rescue => exception
        NotifyInDiscordJob.perform_async("message" => "Unable to index a Customer #{exception.message}")
      end
    end
  end

macwilko avatar Nov 15 '22 08:11 macwilko

I cannot reproduce this issue. Indeed this error:

MeiliSearch::CommunicationError: An error occurred while trying to connect to the MeiliSearch instance: Failed to open TCP connection to localhost:7700 (Connection refused - connect(2) for 127.0.0.1:7700)

does appear but it does not prevent the object from being saved. The callback that is causing this error is an after_commit (or after_save)

            if respond_to?(:after_commit)
              after_commit :ms_perform_index_tasks
            elsif respond_to?(:after_save)
              after_save :ms_perform_index_tasks
            end

Which means that the record will have been persisted before this error occurs.

ellnix avatar Sep 29 '23 09:09 ellnix

So maybe the issue is the error is being raised is actually misleading the user @ellnix?

brunoocasali avatar Oct 09 '23 14:10 brunoocasali

So maybe the issue is the error is being raised is actually misleading the user @ellnix?

Maybe, but this is a meilisearch-ruby error, and I cannot see an alternative to throwing an exception if you fail to connect to your configured MeiliSearch instance.

Another alternative could be to somehow queue all indexing operations and run them when the MeiliSearch instance comes back online. I do not think there is a way we could "wait" for the instance to be back online, but maybe the auto index operation, when it succeeds, could check for tasks that were queued while the search instance was offline.

So something like

Untitled Diagram drawio(1)

The main problem being finding some way to persist tasks, especially since this gem does not necessitate a background queue (and even if it did, most ActiveJob backends are in-memory).

This might be of some small use in production, in case your meilisearch instance is temporarily down and in the meantime there are updates to it that need to happen. In that case you wouldn't have to reindex models with millions of records.

But other than that I can't think of a use case for this in production.

ellnix avatar Oct 09 '23 14:10 ellnix

If we could have a way to opt-in to this feature, it would be an excellent addition. The issue is, like you said, the gem does not require a background queue, and I think it should stay like this.

But thinking more about it, if the user already uses the built-in feature https://github.com/meilisearch/meilisearch-rails/blob/main/README.md#queues--background-jobs I don't think we should even consider the health of meilisearch, because the system will enqueue the index by default, and retry the job if needed.

So I guess there is no need to handle this condition at all right?

brunoocasali avatar Oct 09 '23 19:10 brunoocasali

But thinking more about it, if the user already uses the built-in feature https://github.com/meilisearch/meilisearch-rails/blob/main/README.md#queues--background-jobs I don't think we should even consider the health of meilisearch, because the system will enqueue the index by default, and retry the job if needed.

The problem is that we could have a race condition in this situation where if we have two actions A and B, and A was queued before B it might fail, be retried, and run after B if the instance came back online between A and B. Think of a situation like this:

      it 'will stay deleted' do
        record = { objectId: 123,  title: 'Pride and Prejudice', comment: 'A great book' }
        index.add_documents!(record)

        # instance goes offline
        # user updates record
        record[:title] = 'Crime and Punishment'

        # ActiveJob background task fails and will be retried in 3 seconds
        t = Thread.new do
          sleep 3
          index.update_documents!(record)
        end

        # Instance comes back online
        # user deletes record
        index.delete_document!(123)
        t.join

        expect(index.document(123)).to be_nil
      end

which results in

     Failure/Error: expect(index.document(123)).to be_nil
     
       expected: nil
            got: {"comment"=>"A great book", "objectId"=>123, "title"=>"Crime and Punishment"}

ellnix avatar Oct 13 '23 10:10 ellnix