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

[BUG] MonitorRunners - SearchRequest missing cancelAfterTimeInterval parameter #827

Open
petardz opened this issue Mar 6, 2023 · 4 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@petardz
Copy link
Contributor

petardz commented Mar 6, 2023

What is the bug?
In all monitor runners we don't set cancelAfterTimeInterval parameter on SearchRequest object. Consequence of this is that global cluster setting search.cancel_after_time_interval will override it, which if set too low, could impact monitor run success rate.

How can one reproduce the bug?
Steps to reproduce the behavior:

  1. Set cluster setting search.cancel_after_time_interval to something very low, like 1s
  2. Create monitor on index with some number of documents/shards
  3. See Cancellation timeout error (TaskCancelledExceptioncancelled exception)
@petardz petardz added bug Something isn't working untriaged labels Mar 6, 2023
@rishabhmaurya
Copy link
Collaborator

Are you proposing its resolution to be individual plugin's responsibility to set the search.cancel_after_time_interval with max(cluster setting search.cancel_after_time_interval, plugin's default minimum value say 5s) ?

@petardz
Copy link
Contributor Author

petardz commented Mar 8, 2023

@rishabhmaurya If I understood correctly, cluster setting search.cancel_after_time_interval will be used only if same param in SearchRequest is null?

If that's the case, then plugin can override this setting by setting a cancelAfterTimeInterval param in every SearchRequest before calling _search.

@rishabhmaurya
Copy link
Collaborator

rishabhmaurya commented Mar 9, 2023

I was thinking about the case when search.cancel_after_time_interval is set at cluster with some higher value and the plugin overrides it to a lower value. Thus, max(cluster setting search.cancel_after_time_interval, plugin's default minimum value say 5s) should be used while overriding before calling _search from plugin.
I agree with you that plugins, from where search traffic pattern as a constant function number of index/alerts/other plugins settings, overriding this timeout makes sense. Feel free to raise a PR. Thanks!

@petardz
Copy link
Contributor Author

petardz commented Mar 10, 2023

I was thinking about the case when search.cancel_after_time_interval is set at cluster with some higher value and the plugin overrides it to a lower value. Thus, max(cluster setting search.cancel_after_time_interval, plugin's default minimum value say 5s) should be used while overriding before calling _search from plugin.

Ah got it. That makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants