Add a hard limit for index.number_of_shard
#20682
Conversation
this change adds a hard limit to `index.number_of_shard` that prevents indices from being created that have more than 1024 shards. This is still a huge limit and can only be changed via settins a system property.
s1monw
added
>bug
review
resiliency
:Core/Infra/Settings
| * 99% of the users have less than 1024 shards per index. We also make it a hard check that requires restart of nodes | ||
| * if a cluster should allow to create more than 1024 shards per index. NOTE: this does not limit the number of shards per cluster. | ||
| * this also prevents creating stuff like a new index with millions of shards by accident which essentially kills the entire cluster | ||
| * with OOM on the spot.*/ |
There was a problem hiding this comment.
I feel like this should be reported in documentation too as a WARNING
|
Should this be marked as breaking? 1024 is a ridiculous number, so maybe not. |
not sure I mean you can still set it but it's harder :) |
|
I could go either way honestly. It's configurable, but system properties kind of leave it between a hard and soft limit. With the warning and the sheer number, it should be okay to not. |
| @@ -157,10 +157,24 @@ public static State fromString(String state) { | |||
| } | |||
| } | |||
|
|
|||
| static { | |||
| final int maxNumShards = Integer.parseInt(System.getProperty("index.max_number_of_shards", "1024")); | |||
There was a problem hiding this comment.
I've been trying to prefix the few system properties we have with es; I wonder if we should do this here?
Settings within templates are validated at template creation time, so they will get an error and the template will be rejected. I also double-checked this locally and it prevents it. |
| changed on a closed index. Note: the number of shards are limited to `1024` per | ||
| index. This limitation is a safety limit to prevent accidental creation of indices | ||
| that can destabilize a cluster due to resource allocation. The limit can be modified | ||
| by specifying `-Dindex.max_number_of_shards=128` system property on every node that is |
There was a problem hiding this comment.
maybe it is just me getting confused, but should this be a -D or -E type of parameter?
There was a problem hiding this comment.
Since it's an actual system property, it requires -D. I think the confusion is coming because it's not prefixed with es., so +1 to that.
|
@jpountz @jasontedor @dakrone @javanna @pickypg I wonder if we should get this into 5.1 or rather push it to 5.0 since it's kinda breaking? |
| IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING | ||
| .get(Settings.builder().put("index.number_of_shards", 11).build())); | ||
| assertEquals("Failed to parse value [11] for setting [index.number_of_shards] must be <= " + limit, exception.getMessage()); | ||
| System.clearProperty("es.index.max_number_of_shards"); |
There was a problem hiding this comment.
do we need to do a try/finally so that a failure with this test could not have impact on other tests?
|
I don't mind getting it into 5.0, the risk looks low to me. |
|
I think it's safe to backport this. |
I think #8802 will make the template PUT operation fail. If the template is already there (eg. imported from 2.x), then we will fail at index creation time. |
this change adds a hard limit to `index.number_of_shard` that prevents indices from being created that have more than 1024 shards. This is still a huge limit and can only be changed via settings a system property.
this change adds a hard limit to `index.number_of_shard` that prevents indices from being created that have more than 1024 shards. This is still a huge limit and can only be changed via settings a system property.
this change adds a hard limit to
index.number_of_shardthat preventsindices from being created that have more than 1024 shards. This is still
a huge limit and can only be changed via settings a system property.