Skip to content

Commit

Permalink
Use HashMap in admin get actions (#87938)
Browse files Browse the repository at this point in the history 
Actions for getting aliases, indices, and settings all return a Map. Yet
internally these unnecessarily ImmutableOpenMap. This commit converts
these actions to use HashMap.

relates #86239
  • Loading branch information
@rjernst
rjernst authored Jun 24, 2022
1 parent e3c4cdd commit 981a345
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 35 deletions.
9 changes: 4 additions & 5 deletions ...main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
Expand Down Expand Up @@ -95,7 +94,7 @@ protected void masterOperation(Task task, GetAliasesRequest request, ClusterStat
/**
* Fills alias result with empty entries for requested indices when no specific aliases were requested.
*/
static ImmutableOpenMap<String, List<AliasMetadata>> postProcess(
static Map<String, List<AliasMetadata>> postProcess(
GetAliasesRequest request,
String[] concreteIndices,
Map<String, List<AliasMetadata>> aliases,
Expand All @@ -105,7 +104,7 @@ static ImmutableOpenMap<String, List<AliasMetadata>> postProcess(
SystemIndices systemIndices
) {
boolean noAliasesSpecified = request.getOriginalAliases() == null || request.getOriginalAliases().length == 0;
ImmutableOpenMap.Builder<String, List<AliasMetadata>> mapBuilder = ImmutableOpenMap.builder(aliases);
Map<String, List<AliasMetadata>> mapBuilder = new HashMap<>(aliases);
for (String index : concreteIndices) {
IndexAbstraction ia = state.metadata().getIndicesLookup().get(index);
assert ia.getType() == IndexAbstraction.Type.CONCRETE_INDEX;
Expand All @@ -121,7 +120,7 @@ static ImmutableOpenMap<String, List<AliasMetadata>> postProcess(
assert previous == null;
}
}
final ImmutableOpenMap<String, List<AliasMetadata>> finalResponse = mapBuilder.build();
final Map<String, List<AliasMetadata>> finalResponse = Collections.unmodifiableMap(mapBuilder);
if (systemIndexAccessLevel != SystemIndexAccessLevel.ALL) {
checkSystemIndexAccess(request, systemIndices, state, finalResponse, systemIndexAccessLevel, threadContext);
}
Expand Down Expand Up @@ -155,7 +154,7 @@ private static void checkSystemIndexAccess(
GetAliasesRequest request,
SystemIndices systemIndices,
ClusterState state,
ImmutableOpenMap<String, List<AliasMetadata>> aliasesMap,
Map<String, List<AliasMetadata>> aliasesMap,
SystemIndexAccessLevel systemIndexAccessLevel,
ThreadContext threadContext
) {
Expand Down
30 changes: 15 additions & 15 deletions server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -78,17 +80,15 @@ protected void doMasterOperation(
) {
Map<String, MappingMetadata> mappingsResult = ImmutableOpenMap.of();
Map<String, List<AliasMetadata>> aliasesResult = Map.of();
ImmutableOpenMap<String, Settings> settings = ImmutableOpenMap.of();
ImmutableOpenMap<String, Settings> defaultSettings = ImmutableOpenMap.of();
ImmutableOpenMap<String, String> dataStreams = ImmutableOpenMap.<String, String>builder()
.putAllFromMap(
state.metadata()
.findDataStreams(concreteIndices)
.entrySet()
.stream()
.collect(Collectors.toMap(Map.Entry::getKey, v -> v.getValue().getName()))
)
.build();
Map<String, Settings> settings = Map.of();
Map<String, Settings> defaultSettings = Map.of();
Map<String, String> dataStreams = Map.copyOf(
state.metadata()
.findDataStreams(concreteIndices)
.entrySet()
.stream()
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, v -> v.getValue().getName()))
);
Feature[] features = request.features();
boolean doneAliases = false;
boolean doneMappings = false;
Expand All @@ -111,8 +111,8 @@ protected void doMasterOperation(
break;
case SETTINGS:
if (doneSettings == false) {
ImmutableOpenMap.Builder<String, Settings> settingsMapBuilder = ImmutableOpenMap.builder();
ImmutableOpenMap.Builder<String, Settings> defaultSettingsMapBuilder = ImmutableOpenMap.builder();
Map<String, Settings> settingsMapBuilder = new HashMap<>();
Map<String, Settings> defaultSettingsMapBuilder = new HashMap<>();
for (String index : concreteIndices) {
checkCancellation(task);
Settings indexSettings = state.metadata().index(index).getSettings();
Expand All @@ -127,8 +127,8 @@ protected void doMasterOperation(
defaultSettingsMapBuilder.put(index, defaultIndexSettings);
}
}
settings = settingsMapBuilder.build();
defaultSettings = defaultSettingsMapBuilder.build();
settings = Collections.unmodifiableMap(settingsMapBuilder);
defaultSettings = Collections.unmodifiableMap(defaultSettingsMapBuilder);
doneSettings = true;
}
break;
Expand Down
16 changes: 10 additions & 6 deletions .../java/org/elasticsearch/action/admin/indices/settings/get/TransportGetSettingsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.IndexScopedSettings;
Expand All @@ -29,6 +28,11 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;

import java.util.HashMap;
import java.util.Map;

import static java.util.Collections.unmodifiableMap;

public class TransportGetSettingsAction extends TransportMasterNodeReadAction<GetSettingsRequest, GetSettingsResponse> {

private final SettingsFilter settingsFilter;
Expand Down Expand Up @@ -77,8 +81,8 @@ protected void masterOperation(
ActionListener<GetSettingsResponse> listener
) {
Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, request);
ImmutableOpenMap.Builder<String, Settings> indexToSettingsBuilder = ImmutableOpenMap.builder();
ImmutableOpenMap.Builder<String, Settings> indexToDefaultSettingsBuilder = ImmutableOpenMap.builder();
Map<String, Settings> indexToSettings = new HashMap<>();
Map<String, Settings> indexToDefaultSettings = new HashMap<>();
for (Index concreteIndex : concreteIndices) {
IndexMetadata indexMetadata = state.getMetadata().index(concreteIndex);
if (indexMetadata == null) {
Expand All @@ -94,15 +98,15 @@ protected void masterOperation(
indexSettings = indexSettings.filter(k -> Regex.simpleMatch(request.names(), k));
}

indexToSettingsBuilder.put(concreteIndex.getName(), indexSettings);
indexToSettings.put(concreteIndex.getName(), indexSettings);
if (request.includeDefaults()) {
Settings defaultSettings = settingsFilter.filter(indexScopedSettings.diff(indexSettings, Settings.EMPTY));
if (isFilteredRequest(request)) {
defaultSettings = defaultSettings.filter(k -> Regex.simpleMatch(request.names(), k));
}
indexToDefaultSettingsBuilder.put(concreteIndex.getName(), defaultSettings);
indexToDefaultSettings.put(concreteIndex.getName(), defaultSettings);
}
}
listener.onResponse(new GetSettingsResponse(indexToSettingsBuilder.build(), indexToDefaultSettingsBuilder.build()));
listener.onResponse(new GetSettingsResponse(unmodifiableMap(indexToSettings), unmodifiableMap(indexToDefaultSettings)));
}
}
18 changes: 9 additions & 9 deletions ...java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
Expand All @@ -47,7 +48,7 @@ public void testPostProcess() {
ImmutableOpenMap<String, List<AliasMetadata>> aliases = ImmutableOpenMap.<String, List<AliasMetadata>>builder()
.fPut("b", Collections.singletonList(new AliasMetadata.Builder("y").build()))
.build();
ImmutableOpenMap<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
request,
new String[] { "a", "b", "c" },
aliases,
Expand Down Expand Up @@ -107,7 +108,7 @@ public void testDeprecationWarningEmittedForTotalWildcard() {
.build();
final String[] concreteIndices = { "a", ".b", "c" };
assertEquals(state.metadata().findAliases(request.aliases(), concreteIndices), aliases);
ImmutableOpenMap<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
request,
concreteIndices,
aliases,
Expand Down Expand Up @@ -140,7 +141,7 @@ public void testDeprecationWarningEmittedWhenSystemIndexIsRequested() {
.build();
final String[] concreteIndices = { ".b" };
assertEquals(state.metadata().findAliases(request.aliases(), concreteIndices), aliases);
ImmutableOpenMap<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
request,
concreteIndices,
aliases,
Expand Down Expand Up @@ -170,7 +171,7 @@ public void testDeprecationWarningEmittedWhenSystemIndexIsRequestedByAlias() {
.build();
final String[] concreteIndices = { "a", ".b", "c" };
assertEquals(state.metadata().findAliases(request.aliases(), concreteIndices), aliases);
ImmutableOpenMap<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
request,
concreteIndices,
aliases,
Expand Down Expand Up @@ -200,7 +201,7 @@ public void testDeprecationWarningNotEmittedWhenSystemAccessAllowed() {
.build();
final String[] concreteIndices = { "a", ".b", "c" };
assertEquals(state.metadata().findAliases(request.aliases(), concreteIndices), aliases);
ImmutableOpenMap<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
request,
concreteIndices,
aliases,
Expand All @@ -226,7 +227,7 @@ public void testDeprecationWarningNotEmittedWhenOnlyNonsystemIndexRequested() {
.build();
final String[] concreteIndices = { "c" };
assertEquals(state.metadata().findAliases(request.aliases(), concreteIndices), aliases);
ImmutableOpenMap<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> result = TransportGetAliasesAction.postProcess(
request,
concreteIndices,
aliases,
Expand Down Expand Up @@ -302,11 +303,10 @@ public void testNetNewSystemIndicesDontErrorWhenNotRequested() {
final IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
concreteIndices = indexNameExpressionResolver.concreteIndexNamesWithSystemIndexAccess(clusterState, aliasesRequest);

ImmutableOpenMap<String, List<AliasMetadata>> initialAliases = ImmutableOpenMap.<String, List<AliasMetadata>>builder().build();
ImmutableOpenMap<String, List<AliasMetadata>> finalResponse = TransportGetAliasesAction.postProcess(
Map<String, List<AliasMetadata>> finalResponse = TransportGetAliasesAction.postProcess(
aliasesRequest,
concreteIndices,
initialAliases,
Map.of(),
clusterState,
SystemIndexAccessLevel.NONE,
threadContext,
Expand Down

0 comments on commit 981a345

Please sign in to comment.