Skip to content

Make ILM Steps use Infinite Master Timeout#74143

Merged
original-brownbear merged 3 commits intoelastic:masterfrom
original-brownbear:infinite-timeout-ilm-actions
Jun 17, 2021
Merged

Make ILM Steps use Infinite Master Timeout#74143
original-brownbear merged 3 commits intoelastic:masterfrom
original-brownbear:infinite-timeout-ilm-actions

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

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.
@original-brownbear original-brownbear added >non-issue :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. v8.0.0 v7.14.0 labels Jun 15, 2021
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Jun 15, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
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
Copy Markdown
Member

@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
Copy Markdown
Contributor Author

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

@original-brownbear
Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
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
Copy Markdown
Contributor 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 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 DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. >enhancement Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v7.14.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants