Skip to content

Add a hard limit for index.number_of_shard #20682

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

Merged
merged 6 commits into from
Sep 29, 2016
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,25 @@ public static State fromString(String state) {
}
}

static {
final int maxNumShards = Integer.parseInt(System.getProperty("es.index.max_number_of_shards", "1024"));
if (maxNumShards < 1) {
throw new IllegalArgumentException("es.index.max_number_of_shards must be > 0");
}
MAX_NUMBER_OF_SHARDS = maxNumShards;
}
/* This is a safety limit that should only be exceeded in very rare and special cases. The assumption is that
* 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.*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should be reported in documentation too as a WARNING

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

private static final int MAX_NUMBER_OF_SHARDS;

public static final String INDEX_SETTING_PREFIX = "index.";
public static final String SETTING_NUMBER_OF_SHARDS = "index.number_of_shards";
public static final Setting<Integer> INDEX_NUMBER_OF_SHARDS_SETTING =
Setting.intSetting(SETTING_NUMBER_OF_SHARDS, 5, 1, Property.IndexScope);
Setting.intSetting(SETTING_NUMBER_OF_SHARDS, Math.min(5, MAX_NUMBER_OF_SHARDS), 1, MAX_NUMBER_OF_SHARDS,
Property.IndexScope);
public static final String SETTING_NUMBER_OF_REPLICAS = "index.number_of_replicas";
public static final Setting<Integer> INDEX_NUMBER_OF_REPLICAS_SETTING =
Setting.intSetting(SETTING_NUMBER_OF_REPLICAS, 1, 0, Property.Dynamic, Property.IndexScope);
Expand Down
6 changes: 5 additions & 1 deletion docs/reference/index-modules.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ specific index module:

The number of primary shards that an index should have. Defaults to 5.
This setting can only be set at index creation time. It cannot be
changed on a closed index.
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 `export ES_JAVA_OPTS="-Des.index.max_number_of_shards=128"` system property on every node that is
part of the cluster.

`index.shard.check_on_startup`::
+
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.bootstrap;

import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;

public class EvilSystemPropertyTests extends ESTestCase {

@SuppressForbidden(reason = "manipulates system properties for testing")
public void testMaxNumShards() {
int limit = randomIntBetween(1, 10);
System.setProperty("es.index.max_number_of_shards", Integer.toString(limit));
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to do a try/finally so that a failure with this test could not have impact on other tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
}