From 70f838d0861ba6b7645d3f19339f50b372ffd710 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 31 Jan 2022 21:10:24 +0100 Subject: [PATCH] Speed up Name Collision Check in Metadata.Builder Once either indices, datastreams or their aliases become very numerous, these checks of adding everything to a fresh set and then retaining collisions become very expensive. Slightly adjusted the logic to just collect collisions instead to save endless set adding. Also refactored the logic a little to make it easier to profile the time spent on these validations and extraced some cold-paths for maybe a minor speedup. --- .../cluster/metadata/Metadata.java | 186 +++++++++++------- 1 file changed, 115 insertions(+), 71 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index 437f986c6ad37..f3216ce719086 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -1627,22 +1627,19 @@ public Metadata build(boolean builtIndicesLookupEagerly) { // 1) The datastructures will be rebuilt only when needed. Now during serializing we rebuild these datastructures // while these datastructures aren't even used. // 2) The aliasAndIndexLookup can be updated instead of rebuilding it all the time. - - final Set allIndices = new HashSet<>(indices.size()); final List visibleIndices = new ArrayList<>(); final List allOpenIndices = new ArrayList<>(); final List visibleOpenIndices = new ArrayList<>(); final List allClosedIndices = new ArrayList<>(); final List visibleClosedIndices = new ArrayList<>(); - final Set allAliases = new HashSet<>(); + final Set indicesAliases = new HashSet<>(); final ImmutableOpenMap indicesMap = indices.build(); + final Set allIndices = indicesMap.keySet(); int oldestIndexVersionId = Version.CURRENT.id; for (IndexMetadata indexMetadata : indicesMap.values()) { final String name = indexMetadata.getIndex().getName(); - boolean added = allIndices.add(name); - assert added : "double index named [" + name + "]"; final boolean visible = indexMetadata.isHidden() == false; if (visible) { visibleIndices.add(name); @@ -1658,74 +1655,12 @@ public Metadata build(boolean builtIndicesLookupEagerly) { visibleClosedIndices.add(name); } } - indexMetadata.getAliases().keysIt().forEachRemaining(allAliases::add); + indexMetadata.getAliases().keysIt().forEachRemaining(indicesAliases::add); oldestIndexVersionId = Math.min(oldestIndexVersionId, indexMetadata.getCreationVersion().id); } - final ArrayList duplicates = new ArrayList<>(); - final Set allDataStreams = new HashSet<>(); - DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE); - if (dataStreamMetadata != null) { - for (DataStream dataStream : dataStreamMetadata.dataStreams().values()) { - allDataStreams.add(dataStream.getName()); - } - // Adding data stream aliases: - for (String dataStreamAlias : dataStreamMetadata.getDataStreamAliases().keySet()) { - if (allAliases.add(dataStreamAlias) == false) { - duplicates.add("data stream alias and indices alias have the same name (" + dataStreamAlias + ")"); - } - } - } - - final Set aliasDuplicatesWithIndices = new HashSet<>(allAliases); - aliasDuplicatesWithIndices.retainAll(allIndices); - if (aliasDuplicatesWithIndices.isEmpty() == false) { - // iterate again and constructs a helpful message - for (IndexMetadata cursor : indicesMap.values()) { - for (String alias : aliasDuplicatesWithIndices) { - if (cursor.getAliases().containsKey(alias)) { - duplicates.add(alias + " (alias of " + cursor.getIndex() + ") conflicts with index"); - } - } - } - } - - final Set aliasDuplicatesWithDataStreams = new HashSet<>(allAliases); - aliasDuplicatesWithDataStreams.retainAll(allDataStreams); - if (aliasDuplicatesWithDataStreams.isEmpty() == false) { - // iterate again and constructs a helpful message - for (String alias : aliasDuplicatesWithDataStreams) { - // reported var avoids adding a message twice if an index alias has the same name as a data stream. - boolean reported = false; - for (IndexMetadata cursor : indicesMap.values()) { - if (cursor.getAliases().containsKey(alias)) { - duplicates.add(alias + " (alias of " + cursor.getIndex() + ") conflicts with data stream"); - reported = true; - } - } - // This is for adding an error message for when a data steam alias has the same name as a data stream. - if (reported == false && dataStreamMetadata != null && dataStreamMetadata.dataStreams().containsKey(alias)) { - duplicates.add("data stream alias and data stream have the same name (" + alias + ")"); - } - } - } - - final Set dataStreamDuplicatesWithIndices = new HashSet<>(allDataStreams); - dataStreamDuplicatesWithIndices.retainAll(allIndices); - if (dataStreamDuplicatesWithIndices.isEmpty() == false) { - for (String dataStream : dataStreamDuplicatesWithIndices) { - duplicates.add("data stream [" + dataStream + "] conflicts with index"); - } - } - - if (duplicates.size() > 0) { - throw new IllegalStateException( - "index, alias, and data stream names need to be unique, but the following duplicates " - + "were found [" - + Strings.collectionToCommaDelimitedString(duplicates) - + "]" - ); - } + final DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE); + ensureNoNameCollisions(indicesAliases, indicesMap, allIndices, dataStreamMetadata); SortedMap indicesLookup; if (previousIndicesLookup != null) { @@ -1787,8 +1722,117 @@ public Metadata build(boolean builtIndicesLookupEagerly) { ); } - static SortedMap buildIndicesLookup( + private static void ensureNoNameCollisions( + Set indexAliases, + ImmutableOpenMap indicesMap, + Set allIndices, + @Nullable DataStreamMetadata dataStreamMetadata + ) { + final ArrayList duplicates = new ArrayList<>(); + final Set aliasDuplicatesWithIndices = new HashSet<>(); + for (String alias : indexAliases) { + if (allIndices.contains(alias)) { + aliasDuplicatesWithIndices.add(alias); + } + } + + final Set aliasDuplicatesWithDataStreams = new HashSet<>(); + final Set allDataStreams; + if (dataStreamMetadata != null) { + allDataStreams = dataStreamMetadata.dataStreams().keySet(); + // Adding data stream aliases: + for (String dataStreamAlias : dataStreamMetadata.getDataStreamAliases().keySet()) { + if (indexAliases.contains(dataStreamAlias)) { + duplicates.add("data stream alias and indices alias have the same name (" + dataStreamAlias + ")"); + } + if (allIndices.contains(dataStreamAlias)) { + aliasDuplicatesWithIndices.add(dataStreamAlias); + } + if (allDataStreams.contains(dataStreamAlias)) { + aliasDuplicatesWithDataStreams.add(dataStreamAlias); + } + } + } else { + allDataStreams = Set.of(); + } + + if (aliasDuplicatesWithIndices.isEmpty() == false) { + collectAliasDuplicates(indicesMap, aliasDuplicatesWithIndices, duplicates); + } + + for (String alias : indexAliases) { + if (allDataStreams.contains(alias)) { + aliasDuplicatesWithDataStreams.add(alias); + } + } + if (aliasDuplicatesWithDataStreams.isEmpty() == false) { + collectAliasDuplicates(indicesMap, dataStreamMetadata, aliasDuplicatesWithDataStreams, duplicates); + } + + final Set dataStreamDuplicatesWithIndices = new HashSet<>(); + for (String ds : allDataStreams) { + if (allIndices.contains(ds)) { + dataStreamDuplicatesWithIndices.add(ds); + } + } + for (String dataStream : dataStreamDuplicatesWithIndices) { + duplicates.add("data stream [" + dataStream + "] conflicts with index"); + } + + if (duplicates.isEmpty() == false) { + throw new IllegalStateException( + "index, alias, and data stream names need to be unique, but the following duplicates " + + "were found [" + + Strings.collectionToCommaDelimitedString(duplicates) + + "]" + ); + } + } + + /** + * Iterates the detected duplicates between datastreams and aliases and collects them into the duplicates list as helpful messages. + */ + private static void collectAliasDuplicates( + ImmutableOpenMap indicesMap, DataStreamMetadata dataStreamMetadata, + Set aliasDuplicatesWithDataStreams, + ArrayList duplicates + ) { + for (String alias : aliasDuplicatesWithDataStreams) { + // reported var avoids adding a message twice if an index alias has the same name as a data stream. + boolean reported = false; + for (IndexMetadata cursor : indicesMap.values()) { + if (cursor.getAliases().containsKey(alias)) { + duplicates.add(alias + " (alias of " + cursor.getIndex() + ") conflicts with data stream"); + reported = true; + } + } + // This is for adding an error message for when a data steam alias has the same name as a data stream. + if (reported == false && dataStreamMetadata != null && dataStreamMetadata.dataStreams().containsKey(alias)) { + duplicates.add("data stream alias and data stream have the same name (" + alias + ")"); + } + } + } + + /** + * Collect all duplicate names across indices and aliases that were detected into a list of helpful duplicate failure messages. + */ + private static void collectAliasDuplicates( + ImmutableOpenMap indicesMap, + Set aliasDuplicatesWithIndices, + ArrayList duplicates + ) { + for (IndexMetadata cursor : indicesMap.values()) { + for (String alias : aliasDuplicatesWithIndices) { + if (cursor.getAliases().containsKey(alias)) { + duplicates.add(alias + " (alias of " + cursor.getIndex() + ") conflicts with index"); + } + } + } + } + + static SortedMap buildIndicesLookup( + @Nullable DataStreamMetadata dataStreamMetadata, ImmutableOpenMap indices ) { SortedMap indicesLookup = new TreeMap<>();