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

Make ILM Steps use Infinite Master Timeout #74143

Conversation

original-brownbear
Copy link
Member

Same as #72085 but for ILM. Having a timeout on these internal "requests"
only adds more noise if master is slow already when timed out steps trigger
moves to the error step.
It seems like it is safe to remove the setting for the timeout outright as well
as it was not used anywhere and never documented as far as I can tell.

Same as elastic#72085 but for ILM. Having a timeout on these internal "requests"
only adds more noise if master is slow already when timed out steps trigger
moves to the error step.
It seems like it is safe to remove the setting for the timeout outright as well
as it was not used anywhere and never documented as far as I can tell.
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Jun 15, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this Armin, I'm in general agreement and the code looks good for changing the timeout. However, I'm definitely -1 for removing the setting in 7.x. It's a breaking change and I know that we have recommended the setting to some users in the past (it was actually added at the behest of a user's situation), we shouldn't remove it in 7.x even though it isn't documented.

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Code changes LGTM and I think it's ok to backport to 7.x but I think to be on the safe side we should mark this as an enhancement rather than a non-issue. Not really a breaking change but at least in the backport it'll need to be mentioned as a deprecation in the migration docs and also in the deprecation info API.

@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/packaging-tests-unix-sample (Jenkins issue)

@original-brownbear
Copy link
Member Author

Makes sense brought back the setting now and will document the change in the backport :)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Armin

@original-brownbear
Copy link
Member Author

Thanks Lee + David!

@original-brownbear original-brownbear merged commit 1892489 into elastic:master Jun 17, 2021
@original-brownbear original-brownbear deleted the infinite-timeout-ilm-actions branch June 17, 2021 22:18
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Jun 28, 2021
Same as elastic#72085 but for ILM. Having a timeout on these internal "requests"
only adds more noise if master is slow already when timed out steps trigger
moves to the error step.
It seems like it is safe to remove the setting for the timeout outright as well
as it was not used anywhere and never documented as far as I can tell.
original-brownbear added a commit that referenced this pull request Jun 28, 2021
Same as #72085 but for ILM. Having a timeout on these internal "requests"
only adds more noise if master is slow already when timed out steps trigger
moves to the error step.
It seems like it is safe to remove the setting for the timeout outright as well
as it was not used anywhere and never documented as far as I can tell.
@original-brownbear original-brownbear restored the infinite-timeout-ilm-actions branch April 18, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management >enhancement Team:Data Management Meta label for data/management team v7.14.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants