-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Make ILM Steps use Infinite Master Timeout #74143
Conversation
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.
Pinging @elastic/es-core-features (Team:Core/Features) |
There was a problem hiding this 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.
There was a problem hiding this 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.
Jenkins run elasticsearch-ci/packaging-tests-unix-sample (Jenkins issue) |
Makes sense brought back the setting now and will document the change in the backport :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks Armin
Thanks Lee + David! |
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.
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 #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.