Use Setting infrastructure for number_of_replicas #56712
Conversation
The Setting class provides the ability to define a setting, a default value, along with validation logic. However, there are still many uses of thhe old getAsXXX methods on Settings. This commit converts one of those cases, reading the index number_of_replicas setting, so that the default is not defined in multiple places. relates elastic#56656 relates elastic#56501
rjernst
added
:Data Management/Indices APIs
|
Pinging @elastic/es-core-features (:Core/Features/Indices APIs) |
elasticmachine
added
the
Team:Data Management
| if (numberOfReplicas < 0) { | ||
| throw new IllegalArgumentException("must specify non-negative number of replicas for index [" + index + "]"); | ||
| } | ||
| int numberOfReplicas = INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings); |
There was a problem hiding this comment.
This isn't equivalent, since not having this setting set in the metadata is considered illegal.
There was a problem hiding this comment.
That is, we want to maintain that these are set upstream, and not fallback to defaults here. We want to do that elsewhere, so that if we somehow end up here without it having been set, it's a bug that we ended up with metadata without having it set.
There was a problem hiding this comment.
There were two cases before. The first is the missing case. It was never intended to end up here without number of replicas set; it isn't actually an error from the user's perspective. The default was previously handled upstream, but now we are using the default mechanism in Setting. There's no reason to set number of replicas just so it can be passed in here. The second case was the bounds check, which is also handled in the Setting by the minimum value of 0.
There was a problem hiding this comment.
We copy/rebuild index metadata in various places (shrink, split, clone, restores, etc.). This check ensures that when we rebuild the index metadata, that we have set the number of replicas (and also we do this for the number of shards), instead of relying on the defaults.
There was a problem hiding this comment.
I took at a stab and what I think this should look like in #56801.
|
closing in favor of #56801 |
The Setting class provides the ability to define a setting, a default
value, along with validation logic. However, there are still many uses
of thhe old getAsXXX methods on Settings. This commit converts one of
those cases, reading the index number_of_replicas setting, so that the
default is not defined in multiple places.
relates #56656
relates #56501