Skip to content

Speed up Name Collision Check in Metadata.Builder #83340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 7, 2022
Merged

Speed up Name Collision Check in Metadata.Builder #83340

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 115 additions & 71 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> allIndices = new HashSet<>(indices.size());
final List<String> visibleIndices = new ArrayList<>();
final List<String> allOpenIndices = new ArrayList<>();
final List<String> visibleOpenIndices = new ArrayList<>();
final List<String> allClosedIndices = new ArrayList<>();
final List<String> visibleClosedIndices = new ArrayList<>();
final Set<String> allAliases = new HashSet<>();
final Set<String> indicesAliases = new HashSet<>();
final ImmutableOpenMap<String, IndexMetadata> indicesMap = indices.build();
final Set<String> 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);
Expand All @@ -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<String> duplicates = new ArrayList<>();
final Set<String> 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<String> 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<String> 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<String> 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<String, IndexAbstraction> indicesLookup;
if (previousIndicesLookup != null) {
Expand Down Expand Up @@ -1787,8 +1722,117 @@ public Metadata build(boolean builtIndicesLookupEagerly) {
);
}

static SortedMap<String, IndexAbstraction> buildIndicesLookup(
private static void ensureNoNameCollisions(
Set<String> indexAliases,
ImmutableOpenMap<String, IndexMetadata> indicesMap,
Set<String> allIndices,
@Nullable DataStreamMetadata dataStreamMetadata
) {
final ArrayList<String> duplicates = new ArrayList<>();
final Set<String> aliasDuplicatesWithIndices = new HashSet<>();
for (String alias : indexAliases) {
if (allIndices.contains(alias)) {
aliasDuplicatesWithIndices.add(alias);
}
}

final Set<String> aliasDuplicatesWithDataStreams = new HashSet<>();
final Set<String> 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<String> 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<String, IndexMetadata> indicesMap,
DataStreamMetadata dataStreamMetadata,
Set<String> aliasDuplicatesWithDataStreams,
ArrayList<String> 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<String, IndexMetadata> indicesMap,
Set<String> aliasDuplicatesWithIndices,
ArrayList<String> 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<String, IndexAbstraction> buildIndicesLookup(
@Nullable DataStreamMetadata dataStreamMetadata,
ImmutableOpenMap<String, IndexMetadata> indices
) {
SortedMap<String, IndexAbstraction> indicesLookup = new TreeMap<>();
Expand Down