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

Fix default search timeout in watcher docs #106404

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

nielsbauman
Copy link
Contributor

There are a couple of "default search timeout" settings:

These two do have a default of 30s, but they seem to be influencing other aspects of the watcher module.
The one that actually seems to be the Watcher search input default timeout setting is:

this.defaultTimeout = settings.getAsTime("xpack.watcher.input.search.default_timeout", TimeValue.timeValueMinutes(1));

This seems to have been the case for a while already (the docs line and the default in the code haven't changed since 2017), so then the question becomes until what version we want to backport this (provided that my conclusion above is actually correct).

@nielsbauman nielsbauman added >docs General docs changes :Data Management/Watcher Team:Data Management Meta label for data/management team v8.14.0 labels Mar 18, 2024
Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added the Team:Docs Meta label for docs team label Mar 18, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@masseyke
Copy link
Member

I think you are right that the documentation needs to change to 1m. Given that this is pretty minor and no one has noticed for at least 7 years, I think we only need to put this in main and 7.17.x.
It looks like xpack.watcher.internal.ops.search.default_timeout is the timeout for querying the .watches index (in WatcherService) and the .triggered_watches index (in TriggeredWatchStore), so not related here. The name is a little odd since it's not the default timeout -- it is the timeout for those. But it doesn't seem worth changing that one now.

@masseyke
Copy link
Member

It's also kind of odd that xpack.watcher.input.search.default_timeout is not documented -- our documentation just makes it sound like that default is hard-coded to 1m rather than coming from a setting.

@masseyke
Copy link
Member

Looking in the old private x-pack repo, if the default value of xpack.watcher.input.search.default_timeout was null (which it was before the commit Niels references), then it would fall back on xpack.watcher.internal.ops.search.default_timeout, and in that instance the default of xpack.watcher.internal.ops.search.default_timeout was also 30s. That all changed in 6.0.0. So the documentation has been wrong for 6.5 years.

@nielsbauman nielsbauman added v7.17.19 auto-backport Automatically create backport pull requests when merged labels Mar 19, 2024
@nielsbauman nielsbauman merged commit bceb38d into elastic:main Mar 19, 2024
6 checks passed
@nielsbauman nielsbauman deleted the update-watcher-docs branch March 19, 2024 18:45
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
7.17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Data Management/Watcher >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team v7.17.19 v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants