Skip to content

Commit

Permalink
Guard against improper auto_expand_replica values
Browse files Browse the repository at this point in the history
Previously if the user provided a non-conforming string, it would blow up with
`java.lang.StringIndexOutOfBoundsException: String index out of range: -1`
which is not a *helpful* error message.

Also updated the documentation to make the possible setting values more clear.

Close #5752
  • Loading branch information
mdaniel authored and jpountz committed Jun 6, 2014
1 parent b454f64 commit b0a85f6
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
5 changes: 3 additions & 2 deletions docs/reference/indices/update-settings.asciidoc
Expand Up @@ -34,8 +34,9 @@ settings API:
`index.number_of_replicas`::
The number of replicas each shard has.

`index.auto_expand_replicas`::
Set to an actual value (like `0-all`) or `false` to disable it.
`index.auto_expand_replicas` (string)::
Set to a dash delimited lower and upper bound (e.g. `0-5`)
or one may use `all` as the upper bound (e.g. `0-all`), or `false` to disable it.

`index.blocks.read_only`::
Set to `true` to have the index read only, `false` to allow writes
Expand Down
Expand Up @@ -49,6 +49,9 @@
*/
public class MetaDataUpdateSettingsService extends AbstractComponent implements ClusterStateListener {

// the value we recognize in the "max" position to mean all the nodes
private static final String ALL_NODES_VALUE = "all";

private final ClusterService clusterService;

private final AllocationService allocationService;
Expand All @@ -70,6 +73,8 @@ public void clusterChanged(ClusterChangedEvent event) {
if (!event.state().nodes().localNodeMaster()) {
return;
}
// we will want to know this for translating "all" to a number
final int dataNodeCount = event.state().nodes().dataNodes().size();

Map<Integer, List<String>> nrReplicasChanged = new HashMap<>();

Expand All @@ -78,22 +83,37 @@ public void clusterChanged(ClusterChangedEvent event) {
String autoExpandReplicas = indexMetaData.settings().get(IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS);
if (autoExpandReplicas != null && Booleans.parseBoolean(autoExpandReplicas, true)) { // Booleans only work for false values, just as we want it here
try {
int min;
int max;
final int min;
final int max;

final int dash = autoExpandReplicas.indexOf('-');
if (-1 == dash) {
logger.warn("Unexpected value [{}] for setting [{}]; it should be dash delimited",
autoExpandReplicas, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS);
continue;
}
final String sMin = autoExpandReplicas.substring(0, dash);
try {
min = Integer.parseInt(autoExpandReplicas.substring(0, autoExpandReplicas.indexOf('-')));
String sMax = autoExpandReplicas.substring(autoExpandReplicas.indexOf('-') + 1);
if (sMax.equals("all")) {
max = event.state().nodes().dataNodes().size() - 1;
} else {
min = Integer.parseInt(sMin);
} catch (NumberFormatException e) {
logger.warn("failed to set [{}], minimum value is not a number [{}]",
e, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, sMin);
continue;
}
String sMax = autoExpandReplicas.substring(dash + 1);
if (sMax.equals(ALL_NODES_VALUE)) {
max = dataNodeCount - 1;
} else {
try {
max = Integer.parseInt(sMax);
} catch (NumberFormatException e) {
logger.warn("failed to set [{}], maximum value is neither [{}] nor a number [{}]",
e, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, ALL_NODES_VALUE, sMax);
continue;
}
} catch (Exception e) {
logger.warn("failed to set [{}], wrong format [{}]", e, IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, autoExpandReplicas);
continue;
}

int numberOfReplicas = event.state().nodes().dataNodes().size() - 1;
int numberOfReplicas = dataNodeCount - 1;
if (numberOfReplicas < min) {
numberOfReplicas = min;
} else if (numberOfReplicas > max) {
Expand Down

0 comments on commit b0a85f6

Please sign in to comment.