Skip to content

Commit 8162a65

Browse files
JeremyDahlgrenmridula-s109
authored andcommitted
Avoid extra allocations in RestGetAliasesAction (#126177)
When no explicit aliases are provided in the call there is no need to collect the index names or aliases into HashSets if they won't be used. Also fixed where the index name was being added for each loop of the alias list.
1 parent b5a3a32 commit 8162a65

File tree

2 files changed

+82
-44
lines changed

2 files changed

+82
-44
lines changed

server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java

+60-44
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.xcontent.XContentBuilder;
3737

3838
import java.io.IOException;
39+
import java.util.Collections;
3940
import java.util.HashSet;
4041
import java.util.List;
4142
import java.util.Locale;
@@ -84,59 +85,28 @@ static RestResponse buildRestResponse(
8485
) throws Exception {
8586
final Set<String> indicesToDisplay = new HashSet<>();
8687
final Set<String> returnedAliasNames = new HashSet<>();
87-
for (final Map.Entry<String, List<AliasMetadata>> cursor : responseAliasMap.entrySet()) {
88-
for (final AliasMetadata aliasMetadata : cursor.getValue()) {
89-
if (aliasesExplicitlyRequested) {
88+
if (aliasesExplicitlyRequested) {
89+
for (final Map.Entry<String, List<AliasMetadata>> cursor : responseAliasMap.entrySet()) {
90+
final var aliases = cursor.getValue();
91+
if (aliases.isEmpty() == false) {
9092
// only display indices that have aliases
9193
indicesToDisplay.add(cursor.getKey());
92-
}
93-
returnedAliasNames.add(aliasMetadata.alias());
94-
}
95-
}
96-
dataStreamAliases.entrySet()
97-
.stream()
98-
.flatMap(entry -> entry.getValue().stream())
99-
.forEach(dataStreamAlias -> returnedAliasNames.add(dataStreamAlias.getName()));
100-
101-
// compute explicitly requested aliases that have are not returned in the result
102-
final SortedSet<String> missingAliases = new TreeSet<>();
103-
// first wildcard index, leading "-" as an alias name after this index means
104-
// that it is an exclusion
105-
int firstWildcardIndex = requestedAliases.length;
106-
for (int i = 0; i < requestedAliases.length; i++) {
107-
if (Regex.isSimpleMatchPattern(requestedAliases[i])) {
108-
firstWildcardIndex = i;
109-
break;
110-
}
111-
}
112-
for (int i = 0; i < requestedAliases.length; i++) {
113-
if (Metadata.ALL.equals(requestedAliases[i])
114-
|| Regex.isSimpleMatchPattern(requestedAliases[i])
115-
|| (i > firstWildcardIndex && requestedAliases[i].charAt(0) == '-')) {
116-
// only explicitly requested aliases will be called out as missing (404)
117-
continue;
118-
}
119-
// check if aliases[i] is subsequently excluded
120-
int j = Math.max(i + 1, firstWildcardIndex);
121-
for (; j < requestedAliases.length; j++) {
122-
if (requestedAliases[j].charAt(0) == '-') {
123-
// this is an exclude pattern
124-
if (Regex.simpleMatch(requestedAliases[j].substring(1), requestedAliases[i])
125-
|| Metadata.ALL.equals(requestedAliases[j].substring(1))) {
126-
// aliases[i] is excluded by aliases[j]
127-
break;
94+
for (final AliasMetadata aliasMetadata : aliases) {
95+
returnedAliasNames.add(aliasMetadata.alias());
12896
}
12997
}
13098
}
131-
if (j == requestedAliases.length) {
132-
// explicitly requested aliases[i] is not excluded by any subsequent "-" wildcard in expression
133-
if (false == returnedAliasNames.contains(requestedAliases[i])) {
134-
// aliases[i] is not in the result set
135-
missingAliases.add(requestedAliases[i]);
99+
100+
for (final List<DataStreamAlias> dataStreamAliasList : dataStreamAliases.values()) {
101+
for (final DataStreamAlias dataStreamAlias : dataStreamAliasList) {
102+
returnedAliasNames.add(dataStreamAlias.getName());
136103
}
137104
}
138105
}
139106

107+
// compute explicitly requested aliases that would not be returned in the result
108+
final var missingAliases = computeMissingAliases(requestedAliases, returnedAliasNames);
109+
140110
final RestStatus status;
141111
builder.startObject();
142112
{
@@ -239,4 +209,50 @@ public RestResponse buildResponse(GetAliasesResponse response, XContentBuilder b
239209
});
240210
}
241211

212+
private static SortedSet<String> computeMissingAliases(String[] requestedAliases, Set<String> returnedAliasNames) {
213+
if (requestedAliases.length == 0) {
214+
return Collections.emptySortedSet();
215+
}
216+
217+
final var missingAliases = new TreeSet<String>();
218+
219+
// first wildcard index, leading "-" as an alias name after this index means
220+
// that it is an exclusion
221+
int firstWildcardIndex = requestedAliases.length;
222+
for (int i = 0; i < requestedAliases.length; i++) {
223+
if (Regex.isSimpleMatchPattern(requestedAliases[i])) {
224+
firstWildcardIndex = i;
225+
break;
226+
}
227+
}
228+
for (int i = 0; i < requestedAliases.length; i++) {
229+
if (Metadata.ALL.equals(requestedAliases[i])
230+
|| Regex.isSimpleMatchPattern(requestedAliases[i])
231+
|| (i > firstWildcardIndex && requestedAliases[i].charAt(0) == '-')) {
232+
// only explicitly requested aliases will be called out as missing (404)
233+
continue;
234+
}
235+
// check if aliases[i] is subsequently excluded
236+
int j = Math.max(i + 1, firstWildcardIndex);
237+
for (; j < requestedAliases.length; j++) {
238+
if (requestedAliases[j].charAt(0) == '-') {
239+
// this is an exclude pattern
240+
if (Regex.simpleMatch(requestedAliases[j].substring(1), requestedAliases[i])
241+
|| Metadata.ALL.equals(requestedAliases[j].substring(1))) {
242+
// aliases[i] is excluded by aliases[j]
243+
break;
244+
}
245+
}
246+
}
247+
if (j == requestedAliases.length) {
248+
// explicitly requested aliases[i] is not excluded by any subsequent "-" wildcard in expression
249+
if (false == returnedAliasNames.contains(requestedAliases[i])) {
250+
// aliases[i] is not in the result set
251+
missingAliases.add(requestedAliases[i]);
252+
}
253+
}
254+
}
255+
256+
return missingAliases;
257+
}
242258
}

server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesActionTests.java

+22
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.cluster.metadata.AliasMetadata;
1313
import org.elasticsearch.rest.RestResponse;
1414
import org.elasticsearch.test.ESTestCase;
15+
import org.elasticsearch.test.rest.FakeRestRequest;
1516
import org.elasticsearch.xcontent.XContentBuilder;
1617
import org.elasticsearch.xcontent.XContentFactory;
1718
import org.elasticsearch.xcontent.XContentType;
@@ -51,6 +52,27 @@ public void testBareRequest() throws Exception {
5152
assertThat(restResponse.content().utf8ToString(), equalTo("{\"index\":{\"aliases\":{\"foo\":{},\"foobar\":{}}}}"));
5253
}
5354

55+
public void testNameParamWithAllValue() throws Exception {
56+
final XContentBuilder xContentBuilder = XContentFactory.contentBuilder(XContentType.JSON);
57+
final var req = new FakeRestRequest.Builder(xContentRegistry()).withParams(Map.of("name", "_all")).build();
58+
final RestResponse restResponse = RestGetAliasesAction.buildRestResponse(
59+
req.hasParam("name"),
60+
req.paramAsStringArrayOrEmptyIfAll("name"),
61+
Map.of(
62+
"index",
63+
Arrays.asList(AliasMetadata.builder("foo").build(), AliasMetadata.builder("foobar").build()),
64+
"index2",
65+
List.of()
66+
),
67+
Map.of(),
68+
xContentBuilder
69+
);
70+
assertThat(restResponse.status(), equalTo(OK));
71+
assertThat(restResponse.contentType(), equalTo("application/json"));
72+
// Verify we don't get "index2" since it has no aliases.
73+
assertThat(restResponse.content().utf8ToString(), equalTo("{\"index\":{\"aliases\":{\"foo\":{},\"foobar\":{}}}}"));
74+
}
75+
5476
public void testSimpleAliasWildcardMatchingNothing() throws Exception {
5577
final XContentBuilder xContentBuilder = XContentFactory.contentBuilder(XContentType.JSON);
5678
final RestResponse restResponse = RestGetAliasesAction.buildRestResponse(

0 commit comments

Comments
 (0)