-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Remove legacy role settings #71163
Remove legacy role settings #71163
Conversation
This commit removes the previously deprecated legacy role settings. These settings have been replaced by node.roles.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
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. Just a couple minor suggestions
* `node.voting_only` | ||
|
||
have been removed. Instead, use the `node.roles` setting. If you were previously | ||
using the legacy role settings on 7.13 or later cluster, you will have a |
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.
nit: on a 7.13 or later cluster
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.
I pushed 2362e7c.
|
||
have been removed. Instead, use the `node.roles` setting. If you were previously | ||
using the legacy role settings on 7.13 or later cluster, you will have a | ||
deprecation log message on each of your node informing you of the exact |
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.
Maybe reword the last part of this:
on each of your nodes indicating the exact replacement value for node.roles
.
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.
I pushed 2362e7c.
@@ -20,7 +18,7 @@ | |||
/** | |||
* Represents a node role. | |||
*/ | |||
public abstract class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole> { | |||
public class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole> { |
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.
This could be a followup, but I wonder if we could make this final now. Seems like the only need for override is the enabled by default predicate, but this could be a lambda passed to the second constructor?
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.
Yeah, I will take a close look at opportunities for things like this when I get around to removing the plugin extension point for node roles.
The legacy node.ROLENAME settings were removed in 8.0 (elastic#71163), but machine dependent heap still handles them. This commit cleans up the heap settings determination to no longer look for these legacy settings. relates elastic#85758
This commit removes the previously deprecated legacy role settings. These settings have been replaced by node.roles.
Relates #54998
Relates #71143
Closes #66409