Skip to content

Commit

Permalink
refactor to use new resolveWriteIndex flag instead of IndicesOptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@talevy
talevy committed Jun 25, 2018
1 parent 9e86921 commit df8fabd
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 104 deletions.
24 changes: 15 additions & 9 deletions ...dex/src/test/java/org/elasticsearch/index/reindex/ReindexSourceTargetValidationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@
*/
public class ReindexSourceTargetValidationTests extends ESTestCase {
private static final ClusterState STATE = ClusterState.builder(new ClusterName("test")).metaData(MetaData.builder()
.put(index("target", "target_alias", "target_multi"), true)
.put(index("target2", "target_multi"), true)
.put(index("foo"), true)
.put(index("bar"), true)
.put(index("baz"), true)
.put(index("source", "source_multi"), true)
.put(index("source2", "source_multi"), true)).build();
.put(index("target", false, "target_alias", "target_multi"), true)
.put(index("target2", false, "target_multi"), true)
.put(index("target3", true, "target_multi_with_write"), true)
.put(index("target4", false, "target_multi_with_write"), true)
.put(index("foo", false), true)
.put(index("bar", false), true)
.put(index("baz", false), true)
.put(index("source", false, "source_multi"), true)
.put(index("source2", false, "source_multi"), true)).build();
private static final IndexNameExpressionResolver INDEX_NAME_EXPRESSION_RESOLVER = new IndexNameExpressionResolver(Settings.EMPTY);
private static final AutoCreateIndex AUTO_CREATE_INDEX = new AutoCreateIndex(Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), INDEX_NAME_EXPRESSION_RESOLVER);
Expand Down Expand Up @@ -80,7 +82,11 @@ public void testAliasesContainTarget() {

public void testTargetIsAlias() {
Exception e = expectThrows(IllegalArgumentException.class, () -> succeeds("target_multi", "foo"));
assertThat(e.getMessage(), containsString("Alias [target_multi] points to multiple indices"));
assertThat(e.getMessage(), containsString("Alias [target_multi] has more than one indices associated with it [["));
// The index names can come in either order
assertThat(e.getMessage(), containsString("target"));
assertThat(e.getMessage(), containsString("target2"));
succeeds("target_multi_with_write", "foo"); // should this be allowed?
}

public void testRemoteInfoSkipsValidation() {
Expand All @@ -106,7 +112,7 @@ private void succeeds(RemoteInfo remoteInfo, String target, String... sources) {
INDEX_NAME_EXPRESSION_RESOLVER, AUTO_CREATE_INDEX, STATE);
}

private static IndexMetaData index(String name, String... aliases) {
private static IndexMetaData index(String name, boolean isWriteIndex, String... aliases) {
IndexMetaData.Builder builder = IndexMetaData.builder(name).settings(Settings.builder()
.put("index.version.created", Version.CURRENT.id)
.put("index.number_of_shards", 1)
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ Index getConcreteIndex(String indexOrAlias) {
Index resolveIfAbsent(DocWriteRequest<?> request) {
Index concreteIndex = indices.get(request.index());
if (concreteIndex == null) {
concreteIndex = indexNameExpressionResolver.concreteSingleIndex(state, request);
concreteIndex = indexNameExpressionResolver.concreteWriteIndex(state, request);
indices.put(request.index(), concreteIndex);
}
return concreteIndex;
Expand Down
6 changes: 0 additions & 6 deletions server/src/main/java/org/elasticsearch/action/index/IndexRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.action.CompositeIndicesRequest;
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.RoutingMissingException;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.replication.ReplicatedWriteRequest;
import org.elasticsearch.action.support.replication.ReplicationRequest;
import org.elasticsearch.client.Requests;
Expand Down Expand Up @@ -189,11 +188,6 @@ public ActionRequestValidationException validate() {
return validationException;
}

@Override
public IndicesOptions indicesOptions() {
return IndicesOptions.strictAliasToWriteIndexNoExpandForbidClosed();
}

/**
* The content type. This will be used when generating a document from user provided objects like Maps and when parsing the
* source at index time
Expand Down
22 changes: 1 addition & 21 deletions server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ public enum Option {
IGNORE_ALIASES,
ALLOW_NO_INDICES,
FORBID_ALIASES_TO_MULTIPLE_INDICES,
FORBID_CLOSED_INDICES,
REQUIRE_ALIASES_TO_WRITE_INDEX;
FORBID_CLOSED_INDICES;

public static final EnumSet<Option> NONE = EnumSet.noneOf(Option.class);
}
Expand All @@ -94,9 +93,6 @@ public enum Option {
public static final IndicesOptions STRICT_SINGLE_INDEX_NO_EXPAND_FORBID_CLOSED =
new IndicesOptions(EnumSet.of(Option.FORBID_ALIASES_TO_MULTIPLE_INDICES, Option.FORBID_CLOSED_INDICES),
EnumSet.noneOf(WildcardStates.class));
public static final IndicesOptions STRICT_ALIAS_TO_WRITE_INDEX_NO_EXPAND_FORBID_CLOSED =
new IndicesOptions(EnumSet.of(Option.REQUIRE_ALIASES_TO_WRITE_INDEX, Option.FORBID_CLOSED_INDICES),
EnumSet.noneOf(WildcardStates.class));

private final EnumSet<Option> options;
private final EnumSet<WildcardStates> expandWildcards;
Expand Down Expand Up @@ -235,13 +231,6 @@ public boolean allowAliasesToMultipleIndices() {
return options.contains(Option.FORBID_ALIASES_TO_MULTIPLE_INDICES) == false;
}

/**
* @return whether aliases pointing to a write index should resolve to that index
*/
public boolean requireAliasesToWriteIndex() {
return options.contains(Option.REQUIRE_ALIASES_TO_WRITE_INDEX);
}

/**
* @return whether aliases should be ignored (when resolving a wildcard)
*/
Expand Down Expand Up @@ -386,14 +375,6 @@ public static IndicesOptions strictSingleIndexNoExpandForbidClosed() {
return STRICT_SINGLE_INDEX_NO_EXPAND_FORBID_CLOSED;
}

/**
* @return indices option that requires each specified index or alias to exist, doesn't expand wildcards and
* throws error if any of the aliases resolves to multiple indices with none specified as a write-index
*/
public static IndicesOptions strictAliasToWriteIndexNoExpandForbidClosed() {
return STRICT_ALIAS_TO_WRITE_INDEX_NO_EXPAND_FORBID_CLOSED;
}

/**
* @return indices options that ignores unavailable indices, expands wildcards only to open indices and
* allows that no indices are resolved from wildcard expressions (not returning an error).
Expand Down Expand Up @@ -432,7 +413,6 @@ public String toString() {
", allow_aliases_to_multiple_indices=" + allowAliasesToMultipleIndices() +
", forbid_closed_indices=" + forbidClosedIndices() +
", ignore_aliases=" + ignoreAliases() +
", require_aliases_to_write_index=" + requireAliasesToWriteIndex() +
']';
}
}
6 changes: 0 additions & 6 deletions ...er/src/main/java/org/elasticsearch/action/support/replication/ReplicatedWriteRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.action.bulk.BulkShardRequest;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -59,11 +58,6 @@ public RefreshPolicy getRefreshPolicy() {
return refreshPolicy;
}

@Override
public IndicesOptions indicesOptions() {
return IndicesOptions.strictAliasToWriteIndexNoExpandForbidClosed();
}

@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
Expand Down
6 changes: 0 additions & 6 deletions server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.action.support.replication.ReplicationRequest;
import org.elasticsearch.action.support.single.instance.InstanceShardOperationRequest;
Expand Down Expand Up @@ -170,11 +169,6 @@ public ActionRequestValidationException validate() {
return validationException;
}

@Override
public IndicesOptions indicesOptions() {
return IndicesOptions.strictAliasToWriteIndexNoExpandForbidClosed();
}

/**
* The type of the indexed document.
*/
Expand Down
16 changes: 7 additions & 9 deletions server/src/main/java/org/elasticsearch/cluster/metadata/AliasOrIndex.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,13 @@ void addIndex(IndexMetaData indexMetaData) {
}

public void computeAndValidateWriteIndex() {
final List<IndexMetaData> writeIndices;
if (referenceIndexMetaDatas.size() > 1) {
writeIndices = referenceIndexMetaDatas.stream()
.filter(idxMeta -> Boolean.TRUE.equals(idxMeta.getAliases().get(aliasName).writeIndex()))
.collect(Collectors.toList());
} else if(Boolean.FALSE.equals(referenceIndexMetaDatas.get(0).getAliases().get(aliasName).writeIndex()) == false) {
writeIndices = Collections.singletonList(referenceIndexMetaDatas.get(0));
} else {
writeIndices = Collections.emptyList();
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) {
Expand Down
68 changes: 58 additions & 10 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, S
return concreteIndexNames(context, indexExpressions);
}

/**
/**
* Translates the provided index expression into actual concrete indices, properly deduplicated.
*
* @param state the cluster state containing all the data to resolve to expressions to concrete indices
* @param options defines how the aliases or indices need to be resolved to concrete indices
* @param state the cluster state containing all the data to resolve to expressions to concrete indices
* @param options defines how the aliases or indices need to be resolved to concrete indices
* @param indexExpressions expressions that can be resolved to alias or index names.
* @return the resolved concrete indices based on the cluster state, indices options and index expressions
* @throws IndexNotFoundException if one of the index expressions is pointing to a missing index or alias and the
Expand All @@ -117,7 +117,26 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, S
* indices options in the context don't allow such a case.
*/
public Index[] concreteIndices(ClusterState state, IndicesOptions options, String... indexExpressions) {
Context context = new Context(state, options);
Context context = new Context(state, options, false, false);
return concreteIndices(context, indexExpressions);
}

/**
* Translates the provided index expression into actual concrete indices, properly deduplicated.
*
* @param state the cluster state containing all the data to resolve to expressions to concrete indices
* @param options defines how the aliases or indices need to be resolved to concrete indices
* @param resolveToWriteIndex defines whether to require that aliases resolve to their respective write indices
* @param indexExpressions expressions that can be resolved to alias or index names.
* @return the resolved concrete indices based on the cluster state, indices options and index expressions
* @throws IndexNotFoundException if one of the index expressions is pointing to a missing index or alias and the
* provided indices options in the context don't allow such a case, or if the final result of the indices resolution
* contains no indices and the indices options in the context don't allow such a case.
* @throws IllegalArgumentException if one of the aliases resolve to multiple indices and the provided
* indices options in the context don't allow such a case.
*/
public Index[] concreteIndices(ClusterState state, IndicesOptions options, boolean resolveToWriteIndex, String... indexExpressions) {
Context context = new Context(state, options, false, resolveToWriteIndex);
return concreteIndices(context, indexExpressions);
}

Expand Down Expand Up @@ -195,7 +214,7 @@ Index[] concreteIndices(Context context, String... indexExpressions) {

Collection<IndexMetaData> resolvedIndices = aliasOrIndex.getIndices();

if (aliasOrIndex.isAlias() && options.requireAliasesToWriteIndex()) {
if (aliasOrIndex.isAlias() && context.isResolveToWriteIndex()) {
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) aliasOrIndex;
IndexMetaData writeIndex = alias.getWriteIndex();
if (writeIndex == null) {
Expand Down Expand Up @@ -269,6 +288,24 @@ public Index concreteSingleIndex(ClusterState state, IndicesRequest request) {
return indices[0];
}

/**
* Utility method that allows to resolve an index expression to its corresponding single write index.
*
* @param state the cluster state containing all the data to resolve to expression to a concrete index
* @param request The request that defines how the an alias or an index need to be resolved to a concrete index
* and the expression that can be resolved to an alias or an index name.
* @throws IllegalArgumentException if the index resolution does not lead to an index, or leads to more than one index
* @return the write index obtained as a result of the index resolution
*/
public Index concreteWriteIndex(ClusterState state, IndicesRequest request) {
String indexExpression = request.indices() != null && request.indices().length > 0 ? request.indices()[0] : null;
Index[] indices = concreteIndices(state, request.indicesOptions(), true, indexExpression);
if (indices.length != 1) {
throw new IllegalArgumentException("unable to return a single index as the index and options provided got resolved to multiple indices");
}
return indices[0];
}

/**
* @return whether the specified alias or index exists. If the alias or index contains datemath then that is resolved too.
*/
Expand Down Expand Up @@ -306,7 +343,7 @@ public String[] indexAliases(ClusterState state, String index, Predicate<AliasMe
String... expressions) {
// expand the aliases wildcard
List<String> resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList();
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true);
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false);
for (ExpressionResolver expressionResolver : expressionResolvers) {
resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions);
}
Expand Down Expand Up @@ -526,24 +563,26 @@ static final class Context {
private final IndicesOptions options;
private final long startTime;
private final boolean preserveAliases;
private final boolean resolveToWriteIndex;

Context(ClusterState state, IndicesOptions options) {
this(state, options, System.currentTimeMillis());
}

Context(ClusterState state, IndicesOptions options, boolean preserveAliases) {
this(state, options, System.currentTimeMillis(), preserveAliases);
Context(ClusterState state, IndicesOptions options, boolean preserveAliases, boolean resolveToWriteIndex) {
this(state, options, System.currentTimeMillis(), preserveAliases, resolveToWriteIndex);
}

Context(ClusterState state, IndicesOptions options, long startTime) {
this(state, options, startTime, false);
this(state, options, startTime, false, false);
}

Context(ClusterState state, IndicesOptions options, long startTime, boolean preserveAliases) {
Context(ClusterState state, IndicesOptions options, long startTime, boolean preserveAliases, boolean resolveToWriteIndex) {
this.state = state;
this.options = options;
this.startTime = startTime;
this.preserveAliases = preserveAliases;
this.resolveToWriteIndex = resolveToWriteIndex;
}

public ClusterState getState() {
Expand All @@ -566,6 +605,15 @@ public long getStartTime() {
boolean isPreserveAliases() {
return preserveAliases;
}

/**
* This is used to require that aliases resolve to their write-index. It is currently not used in conjunction
* with <code>preserveAliases</code>.
*/

boolean isResolveToWriteIndex() {
return resolveToWriteIndex;
}
}

private interface ExpressionResolver {
Expand Down
Loading

0 comments on commit df8fabd

Please sign in to comment.