Skip to content

Commit b8fb1eb

Browse files
Make ShardGenerations Immutable (#73452)
This object should be completely immutable. Also added a useful assertion that makes sure we don't accidentally overwrite a valid generation with `null` when dealing dealing failed status updates.
1 parent eb7480e commit b8fb1eb

File tree

1 file changed

+8
-7
lines changed

1 file changed

+8
-7
lines changed

server/src/main/java/org/elasticsearch/repositories/ShardGenerations.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public final class ShardGenerations {
4242
private final Map<IndexId, List<String>> shardGenerations;
4343

4444
private ShardGenerations(Map<IndexId, List<String>> shardGenerations) {
45-
this.shardGenerations = shardGenerations;
45+
this.shardGenerations = Map.copyOf(shardGenerations);
4646
}
4747

4848
private static final Pattern IS_NUMBER = Pattern.compile("^\\d+$");
@@ -76,7 +76,7 @@ public int totalShards() {
7676
* @return indices for which shard generations are tracked
7777
*/
7878
public Collection<IndexId> indices() {
79-
return Collections.unmodifiableSet(shardGenerations.keySet());
79+
return shardGenerations.keySet();
8080
}
8181

8282
/**
@@ -135,8 +135,7 @@ public String getShardGen(IndexId indexId, int shardId) {
135135
}
136136

137137
public List<String> getGens(IndexId indexId) {
138-
final List<String> existing = shardGenerations.get(indexId);
139-
return existing == null ? Collections.emptyList() : Collections.unmodifiableList(existing);
138+
return shardGenerations.getOrDefault(indexId, Collections.emptyList());
140139
}
141140

142141
@Override
@@ -200,15 +199,17 @@ public Builder putAll(ShardGenerations shardGenerations) {
200199
for (int i = 0; i < gens.size(); i++) {
201200
final String gen = gens.get(i);
202201
if (gen != null) {
203-
put(indexId, i, gens.get(i));
202+
put(indexId, i, gen);
204203
}
205204
}
206205
});
207206
return this;
208207
}
209208

210209
public Builder put(IndexId indexId, int shardId, String generation) {
211-
generations.computeIfAbsent(indexId, i -> new HashMap<>()).put(shardId, generation);
210+
String existingGeneration = generations.computeIfAbsent(indexId, i -> new HashMap<>()).put(shardId, generation);
211+
assert generation != null || existingGeneration == null :
212+
"must not overwrite existing generation with null generation [" + existingGeneration + "]";
212213
return this;
213214
}
214215

@@ -223,7 +224,7 @@ public ShardGenerations build() {
223224
// a map entry.
224225
final String[] gens = new String[size];
225226
entry.getValue().forEach((shardId, generation) -> gens[shardId] = generation);
226-
return Arrays.asList(gens);
227+
return Collections.unmodifiableList(Arrays.asList(gens));
227228
}
228229
)));
229230
}

0 commit comments

Comments
 (0)