Skip to content

Commit d2289d2

Browse files
akolarkunnuakolarkunnu
committed
[Metadata Immutability] Change different indices lookup objects from array type to lists
Changed the arrays to immutable List instances, added new versions of the getters which returns List instances. Resolves #8647 Signed-off-by: Abdul Muneer Kolarkunnu <[email protected]>
1 parent 19aedcd commit d2289d2

File tree

11 files changed

+76
-83
lines changed

11 files changed

+76
-83
lines changed

‎CHANGELOG-3.0.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2525
- Add task completion count in search backpressure stats API ([#10028](https://github.com/opensearch-project/OpenSearch/pull/10028/))
2626
- Deprecate CamelCase `PathHierarchy` tokenizer name in favor to lowercase `path_hierarchy` ([#10894](https://github.com/opensearch-project/OpenSearch/pull/10894))
2727
- Breaking change: Do not request "search_pipelines" metrics by default in NodesInfoRequest ([#12497](https://github.com/opensearch-project/OpenSearch/pull/12497))
28+
- Breaking change: Modify the utility APIs in the Metadata to get different indices ([#14723](https://github.com/opensearch-project/OpenSearch/pull/14723))
2829

2930
### Deprecated
3031

‎server/src/main/java/org/opensearch/action/admin/cluster/health/ClusterHealthResponse.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import java.util.Locale;
6161
import java.util.Map;
6262
import java.util.Objects;
63+
import java.util.Set;
6364

6465
import static java.util.Collections.emptyMap;
6566
import static org.opensearch.core.xcontent.ConstructingObjectParser.constructorArg;
@@ -215,13 +216,13 @@ public ClusterHealthResponse(StreamInput in) throws IOException {
215216
}
216217

217218
/** needed for plugins BWC */
218-
public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState) {
219+
public ClusterHealthResponse(String clusterName, Set<String> concreteIndices, ClusterState clusterState) {
219220
this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0));
220221
}
221222

222223
public ClusterHealthResponse(
223224
String clusterName,
224-
String[] concreteIndices,
225+
Set<String> concreteIndices,
225226
ClusterState clusterState,
226227
int numberOfPendingTasks,
227228
int numberOfInFlightFetch,
@@ -239,7 +240,7 @@ public ClusterHealthResponse(
239240

240241
public ClusterHealthResponse(
241242
String clusterName,
242-
String[] concreteIndices,
243+
Set<String> concreteIndices,
243244
ClusterHealthRequest clusterHealthRequest,
244245
ClusterState clusterState,
245246
int numberOfPendingTasks,
@@ -262,7 +263,7 @@ public ClusterHealthResponse(
262263
String clusterName,
263264
ClusterState clusterState,
264265
ClusterSettings clusterSettings,
265-
String[] concreteIndices,
266+
Set<String> concreteIndices,
266267
String awarenessAttributeName,
267268
int numberOfPendingTasks,
268269
int numberOfInFlightFetch,
@@ -286,7 +287,7 @@ public ClusterHealthResponse(
286287
ClusterHealthRequest clusterHealthRequest,
287288
ClusterState clusterState,
288289
ClusterSettings clusterSettings,
289-
String[] concreteIndices,
290+
Set<String> concreteIndices,
290291
String awarenessAttributeName,
291292
int numberOfPendingTasks,
292293
int numberOfInFlightFetch,

‎server/src/main/java/org/opensearch/action/admin/cluster/health/TransportClusterHealthAction.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
import org.opensearch.common.inject.Inject;
5959
import org.opensearch.common.unit.TimeValue;
6060
import org.opensearch.core.action.ActionListener;
61-
import org.opensearch.core.common.Strings;
6261
import org.opensearch.core.common.io.stream.StreamInput;
6362
import org.opensearch.core.common.util.CollectionUtils;
6463
import org.opensearch.discovery.ClusterManagerNotDiscoveredException;
@@ -70,6 +69,8 @@
7069
import org.opensearch.transport.TransportService;
7170

7271
import java.io.IOException;
72+
import java.util.Collections;
73+
import java.util.Set;
7374
import java.util.function.Consumer;
7475
import java.util.function.Predicate;
7576

@@ -494,7 +495,7 @@ private ClusterHealthResponse clusterHealth(
494495
);
495496
}
496497

497-
String[] concreteIndices;
498+
Set<String> concreteIndices;
498499
if (request.level().equals(ClusterHealthRequest.Level.AWARENESS_ATTRIBUTES)) {
499500
String awarenessAttribute = request.getAwarenessAttribute();
500501
concreteIndices = clusterState.getMetadata().getConcreteAllIndices();
@@ -512,12 +513,12 @@ private ClusterHealthResponse clusterHealth(
512513
}
513514

514515
try {
515-
concreteIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request);
516+
concreteIndices = Set.of(indexNameExpressionResolver.concreteIndexNames(clusterState, request));
516517
} catch (IndexNotFoundException e) {
517518
// one of the specified indices is not there - treat it as RED.
518519
ClusterHealthResponse response = new ClusterHealthResponse(
519520
clusterState.getClusterName().value(),
520-
Strings.EMPTY_ARRAY,
521+
Collections.emptySet(),
521522
request,
522523
clusterState,
523524
numberOfPendingTasks,

‎server/src/main/java/org/opensearch/action/admin/indices/datastream/GetDataStreamAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ protected void clusterManagerOperation(Request request, ClusterState state, Acti
336336
}
337337
ClusterStateHealth streamHealth = new ClusterStateHealth(
338338
state,
339-
dataStream.getIndices().stream().map(Index::getName).toArray(String[]::new)
339+
dataStream.getIndices().stream().map(Index::getName).collect(Collectors.toSet())
340340
);
341341
dataStreamInfos.add(new Response.DataStreamInfo(dataStream, streamHealth.getStatus(), indexTemplate));
342342
}

‎server/src/main/java/org/opensearch/cluster/health/ClusterStateHealth.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.util.List;
4949
import java.util.Map;
5050
import java.util.Objects;
51+
import java.util.Set;
5152

5253
/**
5354
* Cluster state health information
@@ -86,9 +87,9 @@ public ClusterStateHealth(final ClusterState clusterState, final ClusterHealthRe
8687
* Creates a new <code>ClusterStateHealth</code> instance considering the current cluster state and the provided index names.
8788
*
8889
* @param clusterState The current cluster state. Must not be null.
89-
* @param concreteIndices An array of index names to consider. Must not be null but may be empty.
90+
* @param concreteIndices A set of index names to consider. Must not be null but may be empty.
9091
*/
91-
public ClusterStateHealth(final ClusterState clusterState, final String[] concreteIndices) {
92+
public ClusterStateHealth(final ClusterState clusterState, final Set<String> concreteIndices) {
9293
numberOfNodes = clusterState.nodes().getSize();
9394
numberOfDataNodes = clusterState.nodes().getDataNodes().size();
9495
hasDiscoveredClusterManager = clusterState.nodes().getClusterManagerNodeId() != null;
@@ -152,7 +153,7 @@ public ClusterStateHealth(final ClusterState clusterState, final String[] concre
152153

153154
public ClusterStateHealth(
154155
final ClusterState clusterState,
155-
final String[] concreteIndices,
156+
final Set<String> concreteIndices,
156157
final ClusterHealthRequest.Level healthLevel
157158
) {
158159
numberOfNodes = clusterState.nodes().getSize();

‎server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ public Map<String, Set<String>> resolveSearchRoutingAllIndices(Metadata metadata
706706
if (routing != null) {
707707
Set<String> r = Sets.newHashSet(Strings.splitStringByCommaToArray(routing));
708708
Map<String, Set<String>> routings = new HashMap<>();
709-
String[] concreteIndices = metadata.getConcreteAllIndices();
709+
Set<String> concreteIndices = metadata.getConcreteAllIndices();
710710
for (String index : concreteIndices) {
711711
routings.put(index, r);
712712
}
@@ -746,7 +746,7 @@ static boolean isExplicitAllPattern(Collection<String> aliasesOrIndices) {
746746
*/
747747
boolean isPatternMatchingAllIndices(Metadata metadata, String[] indicesOrAliases, String[] concreteIndices) {
748748
// if we end up matching on all indices, check, if its a wildcard parameter, or a "-something" structure
749-
if (concreteIndices.length == metadata.getConcreteAllIndices().length && indicesOrAliases.length > 0) {
749+
if (concreteIndices.length == metadata.getConcreteAllIndices().size() && indicesOrAliases.length > 0) {
750750

751751
// we might have something like /-test1,+test1 that would identify all indices
752752
// or something like /-test1 with test1 index missing and IndicesOptions.lenient()
@@ -1182,17 +1182,17 @@ private boolean isEmptyOrTrivialWildcard(List<String> expressions) {
11821182

11831183
private static List<String> resolveEmptyOrTrivialWildcard(IndicesOptions options, Metadata metadata) {
11841184
if (options.expandWildcardsOpen() && options.expandWildcardsClosed() && options.expandWildcardsHidden()) {
1185-
return Arrays.asList(metadata.getConcreteAllIndices());
1185+
return List.copyOf(metadata.getConcreteAllIndices());
11861186
} else if (options.expandWildcardsOpen() && options.expandWildcardsClosed()) {
1187-
return Arrays.asList(metadata.getConcreteVisibleIndices());
1187+
return metadata.getConcreteVisibleIndices();
11881188
} else if (options.expandWildcardsOpen() && options.expandWildcardsHidden()) {
1189-
return Arrays.asList(metadata.getConcreteAllOpenIndices());
1189+
return metadata.getConcreteAllOpenIndices();
11901190
} else if (options.expandWildcardsOpen()) {
1191-
return Arrays.asList(metadata.getConcreteVisibleOpenIndices());
1191+
return metadata.getConcreteVisibleOpenIndices();
11921192
} else if (options.expandWildcardsClosed() && options.expandWildcardsHidden()) {
1193-
return Arrays.asList(metadata.getConcreteAllClosedIndices());
1193+
return metadata.getConcreteAllClosedIndices();
11941194
} else if (options.expandWildcardsClosed()) {
1195-
return Arrays.asList(metadata.getConcreteVisibleClosedIndices());
1195+
return metadata.getConcreteVisibleClosedIndices();
11961196
} else {
11971197
return Collections.emptyList();
11981198
}

‎server/src/main/java/org/opensearch/cluster/metadata/Metadata.java

Lines changed: 36 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -272,12 +272,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio
272272
private final transient int totalNumberOfShards; // Transient ? not serializable anyway?
273273
private final int totalOpenIndexShards;
274274

275-
private final String[] allIndices;
276-
private final String[] visibleIndices;
277-
private final String[] allOpenIndices;
278-
private final String[] visibleOpenIndices;
279-
private final String[] allClosedIndices;
280-
private final String[] visibleClosedIndices;
275+
private final Set<String> allIndices;
276+
private final List<String> visibleIndices;
277+
private final List<String> allOpenIndices;
278+
private final List<String> visibleOpenIndices;
279+
private final List<String> allClosedIndices;
280+
private final List<String> visibleClosedIndices;
281281

282282
private final SortedMap<String, IndexAbstraction> indicesLookup;
283283

@@ -294,12 +294,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio
294294
final Map<String, IndexMetadata> indices,
295295
final Map<String, IndexTemplateMetadata> templates,
296296
final Map<String, Custom> customs,
297-
String[] allIndices,
298-
String[] visibleIndices,
299-
String[] allOpenIndices,
300-
String[] visibleOpenIndices,
301-
String[] allClosedIndices,
302-
String[] visibleClosedIndices,
297+
Set<String> allIndices,
298+
List<String> visibleIndices,
299+
List<String> allOpenIndices,
300+
List<String> visibleOpenIndices,
301+
List<String> allClosedIndices,
302+
List<String> visibleClosedIndices,
303303
SortedMap<String, IndexAbstraction> indicesLookup,
304304
Map<String, SortedMap<Long, String>> systemTemplatesLookup
305305
) {
@@ -325,12 +325,12 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio
325325
this.totalNumberOfShards = totalNumberOfShards;
326326
this.totalOpenIndexShards = totalOpenIndexShards;
327327

328-
this.allIndices = allIndices;
329-
this.visibleIndices = visibleIndices;
330-
this.allOpenIndices = allOpenIndices;
331-
this.visibleOpenIndices = visibleOpenIndices;
332-
this.allClosedIndices = allClosedIndices;
333-
this.visibleClosedIndices = visibleClosedIndices;
328+
this.allIndices = Collections.unmodifiableSet(allIndices);
329+
this.visibleIndices = Collections.unmodifiableList(visibleIndices);
330+
this.allOpenIndices = Collections.unmodifiableList(allOpenIndices);
331+
this.visibleOpenIndices = Collections.unmodifiableList(visibleOpenIndices);
332+
this.allClosedIndices = Collections.unmodifiableList(allClosedIndices);
333+
this.visibleClosedIndices = Collections.unmodifiableList(visibleClosedIndices);
334334
this.indicesLookup = indicesLookup;
335335
this.systemTemplatesLookup = systemTemplatesLookup;
336336
}
@@ -608,42 +608,42 @@ private static String mergePaths(String path, String field) {
608608
/**
609609
* Returns all the concrete indices.
610610
*/
611-
public String[] getConcreteAllIndices() {
611+
public Set<String> getConcreteAllIndices() {
612612
return allIndices;
613613
}
614614

615615
/**
616616
* Returns all the concrete indices that are not hidden.
617617
*/
618-
public String[] getConcreteVisibleIndices() {
618+
public List<String> getConcreteVisibleIndices() {
619619
return visibleIndices;
620620
}
621621

622622
/**
623623
* Returns all of the concrete indices that are open.
624624
*/
625-
public String[] getConcreteAllOpenIndices() {
625+
public List<String> getConcreteAllOpenIndices() {
626626
return allOpenIndices;
627627
}
628628

629629
/**
630630
* Returns all of the concrete indices that are open and not hidden.
631631
*/
632-
public String[] getConcreteVisibleOpenIndices() {
632+
public List<String> getConcreteVisibleOpenIndices() {
633633
return visibleOpenIndices;
634634
}
635635

636636
/**
637637
* Returns all of the concrete indices that are closed.
638638
*/
639-
public String[] getConcreteAllClosedIndices() {
639+
public List<String> getConcreteAllClosedIndices() {
640640
return allClosedIndices;
641641
}
642642

643643
/**
644644
* Returns all of the concrete indices that are closed and not hidden.
645645
*/
646-
public String[] getConcreteVisibleClosedIndices() {
646+
public List<String> getConcreteVisibleClosedIndices() {
647647
return visibleClosedIndices;
648648
}
649649

@@ -1647,12 +1647,12 @@ protected Metadata buildMetadataWithPreviousIndicesLookups() {
16471647
indices,
16481648
templates,
16491649
customs,
1650-
Arrays.copyOf(previousMetadata.allIndices, previousMetadata.allIndices.length),
1651-
Arrays.copyOf(previousMetadata.visibleIndices, previousMetadata.visibleIndices.length),
1652-
Arrays.copyOf(previousMetadata.allOpenIndices, previousMetadata.allOpenIndices.length),
1653-
Arrays.copyOf(previousMetadata.visibleOpenIndices, previousMetadata.visibleOpenIndices.length),
1654-
Arrays.copyOf(previousMetadata.allClosedIndices, previousMetadata.allClosedIndices.length),
1655-
Arrays.copyOf(previousMetadata.visibleClosedIndices, previousMetadata.visibleClosedIndices.length),
1650+
previousMetadata.allIndices,
1651+
previousMetadata.visibleIndices,
1652+
previousMetadata.allOpenIndices,
1653+
previousMetadata.visibleOpenIndices,
1654+
previousMetadata.allClosedIndices,
1655+
previousMetadata.visibleClosedIndices,
16561656
Collections.unmodifiableSortedMap(previousMetadata.indicesLookup),
16571657
systemTemplatesLookup
16581658
);
@@ -1749,17 +1749,6 @@ protected Metadata buildMetadataWithRecomputedIndicesLookups() {
17491749

17501750
validateDataStreams(indicesLookup, (DataStreamMetadata) customs.get(DataStreamMetadata.TYPE));
17511751

1752-
// build all concrete indices arrays:
1753-
// TODO: I think we can remove these arrays. it isn't worth the effort, for operations on all indices.
1754-
// When doing an operation across all indices, most of the time is spent on actually going to all shards and
1755-
// do the required operations, the bottleneck isn't resolving expressions into concrete indices.
1756-
String[] allIndicesArray = allIndices.toArray(Strings.EMPTY_ARRAY);
1757-
String[] visibleIndicesArray = visibleIndices.toArray(Strings.EMPTY_ARRAY);
1758-
String[] allOpenIndicesArray = allOpenIndices.toArray(Strings.EMPTY_ARRAY);
1759-
String[] visibleOpenIndicesArray = visibleOpenIndices.toArray(Strings.EMPTY_ARRAY);
1760-
String[] allClosedIndicesArray = allClosedIndices.toArray(Strings.EMPTY_ARRAY);
1761-
String[] visibleClosedIndicesArray = visibleClosedIndices.toArray(Strings.EMPTY_ARRAY);
1762-
17631752
return new Metadata(
17641753
clusterUUID,
17651754
clusterUUIDCommitted,
@@ -1771,12 +1760,12 @@ protected Metadata buildMetadataWithRecomputedIndicesLookups() {
17711760
indices,
17721761
templates,
17731762
customs,
1774-
allIndicesArray,
1775-
visibleIndicesArray,
1776-
allOpenIndicesArray,
1777-
visibleOpenIndicesArray,
1778-
allClosedIndicesArray,
1779-
visibleClosedIndicesArray,
1763+
allIndices,
1764+
visibleIndices,
1765+
allOpenIndices,
1766+
visibleOpenIndices,
1767+
allClosedIndices,
1768+
visibleClosedIndices,
17801769
indicesLookup,
17811770
systemTemplatesLookup
17821771
);

‎server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ public RemoteRestoreResult restore(
165165
}
166166
} else {
167167
List<String> filteredIndices = filterIndices(
168-
List.of(currentState.metadata().getConcreteAllIndices()),
168+
List.copyOf(currentState.metadata().getConcreteAllIndices()),
169169
indexNames,
170170
IndicesOptions.fromOptions(true, true, true, true)
171171
);

‎server/src/test/java/org/opensearch/action/admin/cluster/health/ClusterHealthResponsesTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import java.util.HashMap;
6363
import java.util.Locale;
6464
import java.util.Map;
65+
import java.util.Set;
6566
import java.util.function.Predicate;
6667
import java.util.regex.Pattern;
6768

@@ -93,7 +94,7 @@ public void testClusterHealth() throws IOException {
9394
TimeValue pendingTaskInQueueTime = TimeValue.timeValueMillis(randomIntBetween(1000, 100000));
9495
ClusterHealthResponse clusterHealth = new ClusterHealthResponse(
9596
"bla",
96-
new String[] { Metadata.ALL },
97+
Set.of(Metadata.ALL),
9798
clusterState,
9899
pendingTasks,
99100
inFlight,
@@ -121,7 +122,7 @@ public void testClusterHealthVerifyClusterManagerNodeDiscovery() throws IOExcept
121122
TimeValue pendingTaskInQueueTime = TimeValue.timeValueMillis(randomIntBetween(1000, 100000));
122123
ClusterHealthResponse clusterHealth = new ClusterHealthResponse(
123124
"bla",
124-
new String[] { Metadata.ALL },
125+
Set.of(Metadata.ALL),
125126
clusterState,
126127
pendingTasks,
127128
inFlight,

0 commit comments

Comments
 (0)