Add a property to mark setting as final #23872
Conversation
This change adds a setting property that sets the value of a setting as final. Updating a final setting is prohibited in any context, for instance an index setting marked as final must be set at index creation and will refuse any update even if the index is closed.
jimczi
added
:Core/Infra/Settings
| @@ -96,6 +96,11 @@ | |||
| Dynamic, | |||
|
|
|||
| /** | |||
| * mark this setting as final (the value cannot be updated) | |||
There was a problem hiding this comment.
I think it is worth explaining here how this is different than not marking the setting as Dynamic.
There was a problem hiding this comment.
If we're looking to prevent updates when an index is closed then we might want to have a setting for that too. That way we can (eventually) require that all settings have one of the three.
I don't like not having one of these meaning "can only be updated on an index and only when that index is closed". If we have three update scenarios we may as well have three properties.
There was a problem hiding this comment.
I think it is worth explaining here how this is different than not marking the setting as Dynamic.
The value cannot be updated in non-dynamic context ? ;)
My understanding is that Dynamic means a setting that can be updated dynamically.
This does not prevent this setting to be updated non-dynamically so I think that Final makes sense as an abstract property in the setting object. Non dynamic contexts are defined at the scope level, so for instance updating the settings on a closed index is considered as non-dynamic. And if we want to add more non-dynamic context we should just ensure that this property is applied ?
There was a problem hiding this comment.
yeah just put this explain part in the comment. we can be explicit and say that it also prevents updates if the index is closed
| @@ -388,6 +388,14 @@ public boolean hasDynamicSetting(String key) { | |||
| } | |||
|
|
|||
| /** | |||
| * Returns <code>true</code> if the setting for the given key is final. Otherwise <code>false</code>. | |||
| */ | |||
| public boolean hasFinalSetting(String key) { | |||
There was a problem hiding this comment.
should we name it isSettingFinal?
| @@ -96,6 +96,11 @@ | |||
| Dynamic, | |||
|
|
|||
| /** | |||
| * mark this setting as final (the value cannot be updated) | |||
There was a problem hiding this comment.
yeah just put this explain part in the comment. we can be explicit and say that it also prevents updates if the index is closed
This change adds a setting property that sets the value of a setting as final. Updating a final setting is prohibited in any context, for instance an index setting marked as final must be set at index creation and will refuse any update even if the index is closed. This change also marks the setting `index.number_of_shards` as Final and the special casing for refusing the updates on this setting has been removed.
* master: Fix Javadocs for BootstrapChecks#enforceLimits Disable bootstrap checks for single-node discovery Add unit tests for the missing aggregator (elastic#23895) Add a property to mark setting as final (elastic#23872) testDifferentRolesMaintainPathOnRestart - fix broken comment testDifferentRolesMaintainPathOnRestart - lower join timeout as split elections are likely Introduce single-node discovery Await termination after shutting down executors
This change adds a setting property
Finalthat sets the value of a setting as final.Updating a
Finalsetting is prohibited in any context, for instance an index settingmarked as final must be set at index creation and will refuse any update even if the index is closed.