Skip to content

Commit 1c1b9ea

Browse files
committed
Clamp auto-expand replicas to the closest value
Ensure that the number of replicas chosen for an auto-expand-able shard is within the range of the available data nodes, i.e., excluding those nodes that cannot be assigned a replica. Closes elastic#84788
1 parent bf9254e commit 1c1b9ea

File tree

2 files changed

+23
-12
lines changed

2 files changed

+23
-12
lines changed

server/src/main/java/org/elasticsearch/cluster/metadata/AutoExpandReplicas.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import java.util.HashMap;
1919
import java.util.List;
2020
import java.util.Map;
21-
import java.util.OptionalInt;
2221
import java.util.function.Supplier;
2322

2423
import static org.elasticsearch.cluster.metadata.MetadataIndexStateService.isIndexVerifiedBeforeClosed;
@@ -101,7 +100,7 @@ public boolean expandToAllNodes() {
101100
return maxReplicas == Integer.MAX_VALUE;
102101
}
103102

104-
public OptionalInt getDesiredNumberOfReplicas(IndexMetadata indexMetadata, RoutingAllocation allocation) {
103+
public int getDesiredNumberOfReplicas(IndexMetadata indexMetadata, RoutingAllocation allocation) {
105104
assert enabled : "should only be called when enabled";
106105
int numMatchingDataNodes = 0;
107106
for (DiscoveryNode discoveryNode : allocation.nodes().getDataNodes().values()) {
@@ -110,20 +109,21 @@ public OptionalInt getDesiredNumberOfReplicas(IndexMetadata indexMetadata, Routi
110109
numMatchingDataNodes++;
111110
}
112111
}
112+
return calculateDesiredNumberOfReplicas(numMatchingDataNodes);
113+
}
113114

115+
// Package private only for testing purposes!
116+
int calculateDesiredNumberOfReplicas(int numMatchingDataNodes) {
114117
final int min = minReplicas();
115118
final int max = getMaxReplicas(numMatchingDataNodes);
116119
int numberOfReplicas = numMatchingDataNodes - 1;
120+
// Make sure number of replicas is always between min and max
117121
if (numberOfReplicas < min) {
118122
numberOfReplicas = min;
119123
} else if (numberOfReplicas > max) {
120124
numberOfReplicas = max;
121125
}
122-
123-
if (numberOfReplicas >= min && numberOfReplicas <= max) {
124-
return OptionalInt.of(numberOfReplicas);
125-
}
126-
return OptionalInt.empty();
126+
return numberOfReplicas;
127127
}
128128

129129
@Override
@@ -153,11 +153,10 @@ public static Map<Integer, List<String>> getAutoExpandReplicaChanges(
153153
if (allocation == null) {
154154
allocation = allocationSupplier.get();
155155
}
156-
autoExpandReplicas.getDesiredNumberOfReplicas(indexMetadata, allocation).ifPresent(numberOfReplicas -> {
157-
if (numberOfReplicas != indexMetadata.getNumberOfReplicas()) {
158-
nrReplicasChanged.computeIfAbsent(numberOfReplicas, ArrayList::new).add(indexMetadata.getIndex().getName());
159-
}
160-
});
156+
int numberOfReplicas = autoExpandReplicas.getDesiredNumberOfReplicas(indexMetadata, allocation);
157+
if (numberOfReplicas != indexMetadata.getNumberOfReplicas()) {
158+
nrReplicasChanged.computeIfAbsent(numberOfReplicas, ArrayList::new).add(indexMetadata.getIndex().getName());
159+
}
161160
}
162161
}
163162
return nrReplicasChanged;

server/src/test/java/org/elasticsearch/cluster/metadata/AutoExpandReplicasTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,16 @@ public void testOnlyAutoExpandAllocationFilteringAfterAllNodesUpgraded() {
282282
terminate(threadPool);
283283
}
284284
}
285+
286+
public void testCalculateDesiredNumberOfReplicas() {
287+
String settingValue = randomFrom("0-all", "0-3");
288+
AutoExpandReplicas autoExpandReplicas = AutoExpandReplicas.SETTING.get(
289+
Settings.builder().put(SETTING_AUTO_EXPAND_REPLICAS, settingValue).build()
290+
);
291+
int max = autoExpandReplicas.maxReplicas();
292+
assertThat(autoExpandReplicas.calculateDesiredNumberOfReplicas(0), equalTo(0));
293+
assertThat(autoExpandReplicas.calculateDesiredNumberOfReplicas(1), equalTo(0));
294+
assertThat(autoExpandReplicas.calculateDesiredNumberOfReplicas(2), equalTo(1));
295+
assertThat(autoExpandReplicas.calculateDesiredNumberOfReplicas(max), equalTo(max - 1));
296+
}
285297
}

0 commit comments

Comments
 (0)