Skip to content

Commit

Permalink
Make the construction of alias abstractions immutable (elastic#66508)
Browse files Browse the repository at this point in the history 
and change validation to be an implementation detail and
part of construction of the alias abstraction.

This change is part of series of changes to clean up the usage
IndexAbstraction.Alias in the codebase, so that it is no longer
needed to cast to IndexAbstraction.Alias and just use the methods
on the IndexAbstraction interface. This should help adding data
stream aliases, so that IndexAbstraction instances of type ALIAS
can also be data stream aliases.

Relates to elastic#66163
  • Loading branch information
@martijnvg
martijnvg committed Jan 6, 2021
1 parent 5d4a439 commit e619fa9
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 74 deletions.
58 changes: 27 additions & 31 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@
*/
package org.elasticsearch.cluster.metadata;

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -180,14 +178,35 @@ class Alias implements IndexAbstraction {

private final String aliasName;
private final List<IndexMetadata> referenceIndexMetadatas;
private final SetOnce<IndexMetadata> writeIndex = new SetOnce<>();
private final IndexMetadata writeIndex;
private final boolean isHidden;

public Alias(AliasMetadata aliasMetadata, IndexMetadata indexMetadata) {
public Alias(AliasMetadata aliasMetadata, List<IndexMetadata> indices) {
this.aliasName = aliasMetadata.getAlias();
this.referenceIndexMetadatas = new ArrayList<>();
this.referenceIndexMetadatas.add(indexMetadata);
this.referenceIndexMetadatas = indices;

List<IndexMetadata> writeIndices = indices.stream()
.filter(idxMeta -> Boolean.TRUE.equals(idxMeta.getAliases().get(aliasName).writeIndex()))
.collect(Collectors.toList());

if (writeIndices.isEmpty() && referenceIndexMetadatas.size() == 1
&& referenceIndexMetadatas.get(0).getAliases().get(aliasName).writeIndex() == null) {
writeIndices.add(referenceIndexMetadatas.get(0));
}

if (writeIndices.size() == 0) {
this.writeIndex = null;
} else if (writeIndices.size() == 1) {
this.writeIndex = writeIndices.get(0);
} else {
List<String> writeIndicesStrings = writeIndices.stream()
.map(i -> i.getIndex().getName()).collect(Collectors.toList());
throw new IllegalStateException("alias [" + aliasName + "] has more than one write index [" +
Strings.collectionToCommaDelimitedString(writeIndicesStrings) + "]");
}

this.isHidden = aliasMetadata.isHidden() == null ? false : aliasMetadata.isHidden();
validateAliasProperties();
}

@Override
Expand All @@ -207,7 +226,7 @@ public List<IndexMetadata> getIndices() {

@Nullable
public IndexMetadata getWriteIndex() {
return writeIndex.get();
return writeIndex;
}

@Override
Expand Down Expand Up @@ -254,30 +273,7 @@ public AliasMetadata getFirstAliasMetadata() {
return referenceIndexMetadatas.get(0).getAliases().get(aliasName);
}

void addIndex(IndexMetadata indexMetadata) {
this.referenceIndexMetadatas.add(indexMetadata);
}

public void computeAndValidateAliasProperties() {
// Validate write indices
List<IndexMetadata> writeIndices = referenceIndexMetadatas.stream()
.filter(idxMeta -> Boolean.TRUE.equals(idxMeta.getAliases().get(aliasName).writeIndex()))
.collect(Collectors.toList());

if (writeIndices.isEmpty() && referenceIndexMetadatas.size() == 1
&& referenceIndexMetadatas.get(0).getAliases().get(aliasName).writeIndex() == null) {
writeIndices.add(referenceIndexMetadatas.get(0));
}

if (writeIndices.size() == 1) {
writeIndex.set(writeIndices.get(0));
} else if (writeIndices.size() > 1) {
List<String> writeIndicesStrings = writeIndices.stream()
.map(i -> i.getIndex().getName()).collect(Collectors.toList());
throw new IllegalStateException("alias [" + aliasName + "] has more than one write index [" +
Strings.collectionToCommaDelimitedString(writeIndicesStrings) + "]");
}

private void validateAliasProperties() {
// Validate hidden status
final Map<Boolean, List<IndexMetadata>> groupedByHiddenStatus = referenceIndexMetadatas.stream()
.collect(Collectors.groupingBy(idxMeta -> Boolean.TRUE.equals(idxMeta.getAliases().get(aliasName).isHidden())));
Expand Down
24 changes: 13 additions & 11 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1514,6 +1514,7 @@ private SortedMap<String, IndexAbstraction> buildIndicesLookup() {
}
}

Map<String, List<IndexMetadata>> aliasToIndices = new HashMap<>();
for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
IndexMetadata indexMetadata = cursor.value;

Expand All @@ -1531,21 +1532,22 @@ private SortedMap<String, IndexAbstraction> buildIndicesLookup() {

for (ObjectObjectCursor<String, AliasMetadata> aliasCursor : indexMetadata.getAliases()) {
AliasMetadata aliasMetadata = aliasCursor.value;
indicesLookup.compute(aliasMetadata.getAlias(), (aliasName, alias) -> {
if (alias == null) {
return new IndexAbstraction.Alias(aliasMetadata, indexMetadata);
} else {
assert alias.getType() == IndexAbstraction.Type.ALIAS : alias.getClass().getName();
((IndexAbstraction.Alias) alias).addIndex(indexMetadata);
return alias;
}
aliasToIndices.compute(aliasMetadata.getAlias(), (aliasName, indices) -> {
if (indices == null) {
indices = new ArrayList<>();
}
indices.add(indexMetadata);
return indices;
});
}
}

indicesLookup.values().stream()
.filter(aliasOrIndex -> aliasOrIndex.getType() == IndexAbstraction.Type.ALIAS)
.forEach(alias -> ((IndexAbstraction.Alias) alias).computeAndValidateAliasProperties());
for (Map.Entry<String, List<IndexMetadata>> entry : aliasToIndices.entrySet()) {
AliasMetadata alias = entry.getValue().get(0).getAliases().get(entry.getKey());
IndexAbstraction existing = indicesLookup.put(entry.getKey(), new IndexAbstraction.Alias(alias, entry.getValue()));
assert existing == null : "duplicate for " + entry.getKey();
}

return indicesLookup;
}

Expand Down
47 changes: 17 additions & 30 deletions server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.collect.List;
import org.elasticsearch.test.ESTestCase;

import java.util.Objects;
Expand All @@ -44,32 +45,23 @@ public void testHiddenAliasValidation() {
IndexMetadata indexWithUnspecifiedAlias = buildIndexWithAlias("nonhidden2", hiddenAliasName, null, Version.CURRENT, false);

{
IndexAbstraction.Alias allHidden = new IndexAbstraction.Alias(hiddenAliasMetadata, hidden1);
allHidden.addIndex(hidden2);
allHidden.addIndex(hidden3);
allHidden.computeAndValidateAliasProperties(); // Should be ok
// Should be ok:
IndexAbstraction.Alias allHidden = new IndexAbstraction.Alias(hiddenAliasMetadata, List.of(hidden1, hidden2, hidden3));
}

{
// Should be ok:
IndexAbstraction.Alias allVisible;
if (randomBoolean()) {
allVisible = new IndexAbstraction.Alias(hiddenAliasMetadata, indexWithNonHiddenAlias);
allVisible.addIndex(indexWithUnspecifiedAlias);
allVisible = new IndexAbstraction.Alias(hiddenAliasMetadata, List.of(indexWithNonHiddenAlias, indexWithUnspecifiedAlias));
} else {
allVisible = new IndexAbstraction.Alias(hiddenAliasMetadata, indexWithUnspecifiedAlias);
allVisible.addIndex(indexWithNonHiddenAlias);
allVisible = new IndexAbstraction.Alias(hiddenAliasMetadata, List.of(indexWithUnspecifiedAlias, indexWithNonHiddenAlias));
}

allVisible.computeAndValidateAliasProperties(); // Should be ok
}

{
IndexAbstraction.Alias oneNonHidden = new IndexAbstraction.Alias(hiddenAliasMetadata, hidden1);
oneNonHidden.addIndex(hidden2);
oneNonHidden.addIndex(hidden3);
oneNonHidden.addIndex(indexWithNonHiddenAlias);
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> oneNonHidden.computeAndValidateAliasProperties());
() -> new IndexAbstraction.Alias(hiddenAliasMetadata, List.of(hidden1, hidden2, hidden3, indexWithNonHiddenAlias)));
assertThat(exception.getMessage(), containsString("alias [" + hiddenAliasName +
"] has is_hidden set to true on indices ["));
assertThat(exception.getMessage(), allOf(containsString(hidden1.getIndex().getName()),
Expand All @@ -80,12 +72,8 @@ public void testHiddenAliasValidation() {
}

{
IndexAbstraction.Alias oneUnspecified = new IndexAbstraction.Alias(hiddenAliasMetadata, hidden1);
oneUnspecified.addIndex(hidden2);
oneUnspecified.addIndex(hidden3);
oneUnspecified.addIndex(indexWithUnspecifiedAlias);
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> oneUnspecified.computeAndValidateAliasProperties());
() -> new IndexAbstraction.Alias(hiddenAliasMetadata, List.of(hidden1, hidden2, hidden3, indexWithUnspecifiedAlias)));
assertThat(exception.getMessage(), containsString("alias [" + hiddenAliasName +
"] has is_hidden set to true on indices ["));
assertThat(exception.getMessage(), allOf(containsString(hidden1.getIndex().getName()),
Expand All @@ -96,18 +84,17 @@ public void testHiddenAliasValidation() {
}

{
IndexAbstraction.Alias mostlyVisibleOneHidden;
if (randomBoolean()) {
mostlyVisibleOneHidden = new IndexAbstraction.Alias(hiddenAliasMetadata, indexWithNonHiddenAlias);
mostlyVisibleOneHidden.addIndex(indexWithUnspecifiedAlias);
} else {
mostlyVisibleOneHidden = new IndexAbstraction.Alias(hiddenAliasMetadata, indexWithUnspecifiedAlias);
mostlyVisibleOneHidden.addIndex(indexWithNonHiddenAlias);
}
final IndexMetadata hiddenIndex = randomFrom(hidden1, hidden2, hidden3);
mostlyVisibleOneHidden.addIndex(hiddenIndex);
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> mostlyVisibleOneHidden.computeAndValidateAliasProperties());
() -> {
if (randomBoolean()) {
new IndexAbstraction.Alias(hiddenAliasMetadata,
List.of(indexWithNonHiddenAlias, indexWithUnspecifiedAlias, hiddenIndex));
} else {
new IndexAbstraction.Alias(hiddenAliasMetadata,
List.of(indexWithUnspecifiedAlias, indexWithNonHiddenAlias, hiddenIndex));
}
});
assertThat(exception.getMessage(), containsString("alias [" + hiddenAliasName + "] has is_hidden set to true on " +
"indices [" + hiddenIndex.getIndex().getName() + "] but does not have is_hidden set to true on indices ["));
assertThat(exception.getMessage(), allOf(containsString(indexWithUnspecifiedAlias.getIndex().getName()),
Expand Down
9 changes: 7 additions & 2 deletions ...n/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherIndexingListenerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.cluster.routing.ShardRoutingState;
import org.elasticsearch.cluster.routing.TestShardRouting;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.time.DateUtils;
import org.elasticsearch.index.Index;
Expand Down Expand Up @@ -719,8 +720,12 @@ private ClusterState mockClusterState(String watchIndex) {
// now point the alias, if the watch index is not .watches
if (watchIndex.equals(Watch.INDEX) == false) {
AliasMetadata aliasMetadata = mock(AliasMetadata.class);
when(aliasMetadata.alias()).thenReturn(watchIndex);
indices.put(Watch.INDEX, new IndexAbstraction.Alias(aliasMetadata, indexMetadata));
when(aliasMetadata.writeIndex()).thenReturn(true);
when(aliasMetadata.getAlias()).thenReturn(Watch.INDEX);
ImmutableOpenMap.Builder<String, AliasMetadata> aliases = ImmutableOpenMap.builder();
aliases.put(Watch.INDEX, aliasMetadata);
when(indexMetadata.getAliases()).thenReturn(aliases.build());
indices.put(Watch.INDEX, new IndexAbstraction.Alias(aliasMetadata, Collections.singletonList(indexMetadata)));
}

when(metadata.getIndicesLookup()).thenReturn(indices);
Expand Down

0 comments on commit e619fa9

Please sign in to comment.