-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Add deprecation warning for default shards #30587
Add deprecation warning for default shards #30587
Conversation
This commit adds a deprecation warning to 6.x inform users that the default number of shards is changing from 5 to 1. To avoid burdening our REST tests with what would effectively be a warning assertion on every REST test, we automatically assume that such warning headers are expected. However, we add a dedicated test to ensure that the warning header comes back when expected (and does not come back when it is not expected).
Pinging @elastic/es-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
@@ -376,6 +371,9 @@ public ClusterState execute(ClusterState currentState) throws Exception { | |||
// now, put the request settings, so they override templates | |||
indexSettingsBuilder.put(request.settings()); | |||
if (indexSettingsBuilder.get(SETTING_NUMBER_OF_SHARDS) == null) { | |||
deprecationLogger.deprecated("the default number of shards will change from [5] to [1] in 7.0.0; " |
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: I think deprecation messages usually begin with a capital letter? Looks that way in most every other place I looked at a quick glance.
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.
It's a mix. I dislike them for exception and log messages. It especially grates on me to see the capitalization without a period (as we seem prone to do).
This reverts commit 3bb200f.
* 6.x: Revert "Silence IndexUpgradeIT test failures. (#30430)" [DOCS] Remove references to changelog and to highlights Revert "Mute ML upgrade test (#30458)" [ML] Fix BWC version for backport of #30125 [Docs] Improve section detailing translog usage (#30573) [Tests] Relax allowed delta in extended_stats aggregation (#30569) Fail if reading from closed KeyStoreWrapper (#30394) [ML] Reverse engineer Grok patterns from categorization results (#30125) Derive max composite buffers from max content len Update build file due to doc file rename SQL: Extract SQL request and response classes (#30457) Remove the changelog (#30593) Revert "Add deprecation warning for default shards (#30587)" Silence IndexUpgradeIT test failures. (#30430) Add deprecation warning for default shards (#30587) [DOCS] Adds 6.4.0 release highlight pages [DOCS] Adds release highlight pages (#30590) Docs: Document how to rebuild analyzers (#30498) [DOCS] Fixes title capitalization in security content LLRest: Add equals and hashcode tests for Request (#30584) [DOCS] Fix realm setting names (#30499) [DOCS] Fix path info for various security files (#30502) Docs: document precision limitations of geo_bounding_box (#30540) Fix non existing javadocs link in RestClientTests Auto-expand replicas only after failing nodes (#30553)
…nfig With Elasticsearch 6 (>6.4), we have a warning on index creation : the default number of shards will change from [5] to [1] in 7.0.0 See elastic/elasticsearch#30587 I propose to add number_of_shards in index config. Also add number_of_replicas that is better explicit. In case on only one node, it must be 0. Test plan : 1) Use Elasticsearch 2) Apply patch and flush memcached 3) Rebuild indexes : misc/search_tools/rebuild_elasticsearch.pl -v -b -d 4) Check you dont have a warning about number of shards 5) Check the settings of index : curl '<cluster>:9200/<myindex>_biblios/_settings?pretty&filter_path=**.number_of_*' 6) You should see : "number_of_shards" : "5", "number_of_replicas" : "1" Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
…nfig With Elasticsearch 6 (>6.4), we have a warning on index creation : the default number of shards will change from [5] to [1] in 7.0.0 See elastic/elasticsearch#30587 I propose to add number_of_shards in index config. Also add number_of_replicas that is better explicit. In case on only one node, it must be 0. Test plan : 1) Use Elasticsearch 2) Apply patch and flush memcached 3) Rebuild indexes : misc/search_tools/rebuild_elasticsearch.pl -v -b -d 4) Check you dont have a warning about number of shards 5) Check the settings of index : curl '<cluster>:9200/<myindex>_biblios/_settings?pretty&filter_path=**.number_of_*' 6) You should see : "number_of_shards" : "5", "number_of_replicas" : "1" Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> (cherry picked from commit 3dc90c6) Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
…nfig With Elasticsearch 6 (>6.4), we have a warning on index creation : the default number of shards will change from [5] to [1] in 7.0.0 See elastic/elasticsearch#30587 I propose to add number_of_shards in index config. Also add number_of_replicas that is better explicit. In case on only one node, it must be 0. Test plan : 1) Use Elasticsearch 2) Apply patch and flush memcached 3) Rebuild indexes : misc/search_tools/rebuild_elasticsearch.pl -v -b -d 4) Check you dont have a warning about number of shards 5) Check the settings of index : curl '<cluster>:9200/<myindex>_biblios/_settings?pretty&filter_path=**.number_of_*' 6) You should see : "number_of_shards" : "5", "number_of_replicas" : "1" Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> (cherry picked from commit 3dc90c6) Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> (cherry picked from commit 31b3763) Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
…nfig With Elasticsearch 6 (>6.4), we have a warning on index creation : the default number of shards will change from [5] to [1] in 7.0.0 See elastic/elasticsearch#30587 I propose to add number_of_shards in index config. Also add number_of_replicas that is better explicit. In case on only one node, it must be 0. Test plan : 1) Use Elasticsearch 2) Apply patch and flush memcached 3) Rebuild indexes : misc/search_tools/rebuild_elasticsearch.pl -v -b -d 4) Check you dont have a warning about number of shards 5) Check the settings of index : curl '<cluster>:9200/<myindex>_biblios/_settings?pretty&filter_path=**.number_of_*' 6) You should see : "number_of_shards" : "5", "number_of_replicas" : "1" Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> (cherry picked from commit 3dc90c6) Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> (cherry picked from commit 31b3763) Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> (cherry picked from commit ce3d0ff) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
This commit adds a deprecation warning to 6.x inform users that the default number of shards is changing from 5 to 1. To avoid burdening our REST tests with what would effectively be a warning assertion on every REST test, we automatically assume that such warning headers are expected. However, we add a dedicated test to ensure that the warning header comes back when expected (and does not come back when it is not expected).
Relates #30539