Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent model callbacks if deactivated #1

Open
wants to merge 2 commits into
base: prevent_enqueue_if_deactivated
Choose a base branch
from

Conversation

pozelli
Copy link

@pozelli pozelli commented Mar 9, 2025

I realized later that, in fact, Meilisearch was disabled in my application, but since I didn't restart Sidekiq, it was still active there. As the application added items to the queue, Sidekiq attempted to communicate with the Meilisearch server, causing connection errors.

However, your PR helps a lot in my case, as it prevents the items from unnecessarily consuming Sidekiq's processing.

Finally, all of this made me reflect that it might be even better and cleaner to disable, in the first place, the callbacks added by meilisearch-rails.

Copy link
Owner

@ellnix ellnix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed what I hope is a helpful test for you to experiment. It's normally frowned upon to push on top of someone else's work, so feel free to delete my commit if you choose :)

@@ -418,7 +418,7 @@ def meilisearch(options = {}, &block)
proc.call(record, remove) if ::MeiliSearch::Rails.active? && !ms_without_auto_index_scope
end
end
unless options[:auto_index] == false
if ::MeiliSearch::Rails.active? && options[:auto_index] != false
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually no way to prevent creating callbacks when meilisearch is disabled without significant restructuring.

The problem lies in the fact that Meilisearch can be deactivated after it was started as active. The active configuration is dynamic.

So while this change avoids creating the callbacks when Meilisearch is started in an inactive state, it does not prevent the callbacks from being run when Meilisearch starts enabled and then gets disabled.

It also does not have a mechanism to add the callbacks when Meilisearch starts disabled and gets enabled.

Essentially, since the active state is dynamic, it makes sense to be lazy and defer deciding whether or not to index during the callback even if it incurs a performance cost.

You can add this check inside the callback but I would not expect a large performance difference since when Meilisearch is disabled every method call on it is basically a noop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants