Disabled meilisearch-rails but it still triggers the configured background jobs

[pozelli]

Description

When I disable Meilisearch using the active parameter:

Meilisearch::Rails.configuration = {
  ...
  active: false
}

It is not completely disabled. It still triggers the background jobs if the enqueue option is set:

meilisearch enqueue: :trigger_sidekiq_job

Expected behavior

It should not trigger the background job.

Current behavior

It triggers the background job.

Environment (please complete the following information):

• OS: Ubuntu
• Meilisearch server version: 1.11.0
• meilisearch-rails version: 0.14.2

[ellnix]

From reading the source code, I'm not sure this is unintended behavior. The active option in the config disables any requests from going out to the meilisearch server but leaves everything working as-is. I think your request is reasonable though so I'll create a PR and see what everyone else thinks.

[pozelli]

From reading the source code, I'm not sure this is unintended behavior. The active option in the config disables any requests from going out to the meilisearch server but leaves everything working as-is. I think your request is reasonable though so I'll create a PR and see what everyone else thinks.

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

However, your PR already 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, if disabled.

Please check: ellnix#1

[ellnix]

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

I can imagine that could be really inefficient, especially if sidekiq is configured to retry on failure (though I'm not sure sidekiq would realize that the indexing is failing even)

Please check: ellnix#1

I'll paste my review of the change in that PR for anyone going through this discussion in the future. The PR makes a change to only create callbacks on models when meilisearch is active.

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.

[pozelli]

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

I can imagine that could be really inefficient, especially if sidekiq is configured to retry on failure (though I'm not sure sidekiq would realize that the indexing is failing even)

Please check: ellnix#1

I'll paste my review of the change in that PR for anyone going through this discussion in the future. The PR makes a change to only create callbacks on models when meilisearch is active.

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.

Thank you for the review! I hadn't thought about that case. In my specific situation, it doesn't make sense to toggle Meilisearch on and off at runtime, so disabling the callbacks when it's inactive is actually useful. However, I understand that in a scenario where this configuration changes dynamically, the approach would need to be different.

[ellnix]

disabling the callbacks when it's inactive is actually useful

Have you quantified it in some way? The inactive callbacks are not that computationally expensive, so I'm having a hard time seeing why that would be the case. If there's a benefit to disabling callbacks we could have a separate config field for disabling callbacks.

[pozelli]

disabling the callbacks when it's inactive is actually useful

Have you quantified it in some way? The inactive callbacks are not that computationally expensive, so I'm having a hard time seeing why that would be the case. If there's a benefit to disabling callbacks we could have a separate config field for disabling callbacks.

I haven't measured it, but I just prefer to avoid dead ends. I know it's not the original purpose, but I’d rather treat the active flag more strictly—preventing meilisearch-rails from interfering with the execution flow.