Deprecate updating replica count with auto-expand #73230
Conversation
Today if you update `index.number_of_replicas` on indices that have `index.auto_expand_replicas` set then the update is accepted but silently ignored. Lenience like this is surprising. This commit deprecates this behaviour so that it can be removed in a future release. Relates elastic#27835
DaveCTurner
added
>deprecation
:Distributed/Distributed
|
Pinging @elastic/es-distributed (Team:Distributed) |
| @@ -756,7 +756,6 @@ public synchronized void updateMetadata(final IndexMetadata currentIndexMetadata | |||
| if (currentSettingsVersion == newSettingsVersion) { | |||
| assert updateIndexSettings == false; | |||
| } else { | |||
| assert updateIndexSettings; | |||
There was a problem hiding this comment.
This assertion almost holds but fails in cases where you set index.number_of_replicas and then auto-expand replicas sets it back again. I think it's ok to drop it, it's not really a big deal to process the occasional no-op update.
| @@ -130,6 +135,31 @@ public ClusterState execute(ClusterState currentState) { | |||
| } | |||
|
|
|||
| if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(openSettings)) { | |||
|
|
|||
| final boolean updatingReplicasHasEffect; | |||
| if (IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.exists(openSettings)) { | |||
There was a problem hiding this comment.
NB this doesn't account for updates that use wildcards, but nor does the rest of this logic.
|
I apparently failed to include a top-level review comment, here's what I meant to say: I'm not 100% set on this direction, we could reasonably argue that today's behaviour is preferred and therefore close #27835 maybe with some docs changes. We permit other kinds of no-op updates, removing settings that are not even set, or targeting zero indices with a wildcard, and maybe making this call fail will just be painful for clients. It's not like we've seen a lot of interest in fixing this since 2017. Thought it'd be useful to pin down exactly what would be needed to resolve the issue anyway. |
|
Hi @DaveCTurner, I've created a changelog YAML for you. Note that since this PR is labelled |
DaveCTurner
removed
the
:Distributed/Distributed
DaveCTurner
added
the
:Distributed/Allocation
Today if you update
index.number_of_replicason indices that haveindex.auto_expand_replicasset then the update is accepted butsilently ignored. Lenience like this is surprising. This commit
deprecates this behaviour so that it can be removed in a future release.
Relates #27835