-
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
Deprecations for single data node setting #73733
Deprecations for single data node setting #73733
Conversation
In elastic#55805, we added a setting to allow single data node clusters to respect the high watermark. This commit adds deprecation warnings to 7.x to ensure we can make only true valid in 8.0.
Pinging @elastic/es-distributed (Team:Distributed) |
In elastic#55805, we added a setting to allow single data node clusters to respect the high watermark. In elastic#73733 we added the related deprecations. This commit ensures the only valid value for the setting is true and adds deprecations if the setting is set. The setting will be removed in a future release.
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.
Similarly to #73737 (comment), if the cluster has a single data node then should we emit deprecation logs even if cluster.routing.allocation.disk.watermark.enable_for_single_data_node
is unset?
I think such a deprecation warning would be annoying to 99%+ of users. I wonder if a compromise could be to let the deprecation info check depend on number of data nodes in the cluster and emit the deprecation issue if the setting is unset in a single data node cluster? |
++ that's what I meant by
😁 |
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; I left a docs suggestion and another nit but this doesn't need another round.
@@ -41,7 +44,7 @@ | |||
|
|||
class NodeDeprecationChecks { | |||
|
|||
static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules) { | |||
static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules, ClusterState cs) { |
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, for consistency I'd prefer this (and similarly below too)
static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules, ClusterState cs) { | |
static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules, final ClusterState clusterState) { |
I'm ok with state
instead of clusterState
although I prefer the latter.
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 agree, I was too lazy.....
Co-authored-by: David Turner <[email protected]>
Thanks David! |
In #55805, we added a setting to allow single data node clusters to respect the high watermark. In #73733 we added the related deprecations. This commit ensures the only valid value for the setting is true and adds deprecations if the setting is set. The setting will be removed in a future release. Co-authored-by: David Turner <[email protected]>
The deprecation warning for relying on the default value for `cluster.routing.allocation.disk.watermark.enable_for_single_data_node` was marked critical, but upgrading should never fail due to this. Now changed to a warning. Relates elastic#73733
The deprecation warning for relying on the default value for `cluster.routing.allocation.disk.watermark.enable_for_single_data_node` was marked critical, but upgrading should never fail due to this. Now changed to a warning. Relates #73733
The deprecation warning for relying on the default value for `cluster.routing.allocation.disk.watermark.enable_for_single_data_node` was marked critical, but upgrading should never fail due to this. Now changed to a warning. Relates #73733
…ata_node` setting Prior to 7.8, whenever a cluster had only a single data node, the watermarks would not be respected. This was incompatible with how storage based autoscaling on ESS/ECH works and in order to change this in 7.8+ in a backwards compatible way, we had to introduce the `cluster.routing.allocation.disk.watermark.enable_for_single_data_node node` setting. The setting was deprecated in elastic#73733 (7.14), and was made to accept only `true` in elastic#73737 (8.0).
…ata_node` setting Prior to 7.8, whenever a cluster had only a single data node, the watermarks would not be respected. This was incompatible with how storage based autoscaling on ESS/ECH works and in order to change this in 7.8+ in a backwards compatible way, we had to introduce the `cluster.routing.allocation.disk.watermark.enable_for_single_data_node node` setting. The setting was deprecated in elastic#73733 (7.14), and was made to accept only `true` in elastic#73737 (8.0).
…ata_node` setting (#114207) Prior to 7.8, whenever a cluster had only a single data node, the watermarks would not be respected. This was incompatible with how storage based autoscaling works and in order to change this in 7.8+ in a backwards compatible way, we had to introduce the `cluster.routing.allocation.disk.watermark.enable_for_single_data_node node` setting. The setting was deprecated in #73733 (7.14), and was made to accept only `true` in #73737 (8.0).
…ata_node` setting (#114207) Prior to 7.8, whenever a cluster had only a single data node, the watermarks would not be respected. This was incompatible with how storage based autoscaling works and in order to change this in 7.8+ in a backwards compatible way, we had to introduce the `cluster.routing.allocation.disk.watermark.enable_for_single_data_node node` setting. The setting was deprecated in #73733 (7.14), and was made to accept only `true` in #73737 (8.0).
…ata_node` setting (elastic#114207) Prior to 7.8, whenever a cluster had only a single data node, the watermarks would not be respected. This was incompatible with how storage based autoscaling works and in order to change this in 7.8+ in a backwards compatible way, we had to introduce the `cluster.routing.allocation.disk.watermark.enable_for_single_data_node node` setting. The setting was deprecated in elastic#73733 (7.14), and was made to accept only `true` in elastic#73737 (8.0).
In #55805, we added a setting to allow single data node clusters to
respect the high watermark. This commit adds deprecation warnings to 7.x
to ensure we can make only true valid in 8.0.