Skip to content

Improve error message when aliases are not supported #25728

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
javanna merged 1 commit into from
Jul 18, 2017
Merged

Improve error message when aliases are not supported #25728

Show file tree
Hide file tree
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
39 changes: 26 additions & 13 deletions core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,20 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
final Set<Index> concreteIndices = new HashSet<>(expressions.size());
for (String expression : expressions) {
AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(expression);
if (aliasOrIndex == null || (aliasOrIndex.isAlias() && context.getOptions().ignoreAliases())) {
if (aliasOrIndex == null ) {
if (failNoIndices) {
IndexNotFoundException infe = new IndexNotFoundException(expression);
infe.setResources("index_expression", expression);
throw infe;
} else {
continue;
}
} else if (aliasOrIndex.isAlias() && context.getOptions().ignoreAliases()) {
if (failNoIndices) {
throw aliasesNotSupportedException(expression);
} else {
continue;
}
}

Collection<IndexMetaData> resolvedIndices = aliasOrIndex.getIndices();
Expand All @@ -192,7 +198,8 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
for (IndexMetaData indexMetaData : resolvedIndices) {
indexNames[i++] = indexMetaData.getIndex().getName();
}
throw new IllegalArgumentException("Alias [" + expression + "] has more than one indices associated with it [" + Arrays.toString(indexNames) + "], can't execute a single index op");
throw new IllegalArgumentException("Alias [" + expression + "] has more than one indices associated with it [" +
Arrays.toString(indexNames) + "], can't execute a single index op");
}

for (IndexMetaData index : resolvedIndices) {
Expand Down Expand Up @@ -220,6 +227,11 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
return concreteIndices.toArray(new Index[concreteIndices.size()]);
}

private static IllegalArgumentException aliasesNotSupportedException(String expression) {
return new IllegalArgumentException("The provided expression [" + expression + "] matches an " +
"alias, specify the corresponding concrete indices instead.");
Copy link
Contributor

@olcbean olcbean Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more in sync with the majority of ES msgs if indices is replaced with index in specify the corresponding concrete indices instead.

Copy link
Member Author

@javanna javanna Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it can be more than one index as an alias can point to multiple indices. Would you change that anyway?

}

/**
* Utility method that allows to resolve an index expression to its corresponding single concrete index.
* Callers should make sure they provide proper {@link org.elasticsearch.action.support.IndicesOptions}
Expand Down Expand Up @@ -270,8 +282,7 @@ public String[] filteringAliases(ClusterState state, String index, String... exp
}

/**
* Iterates through the list of indices and selects the effective list of required aliases for the
* given index.
* Iterates through the list of indices and selects the effective list of required aliases for the given index.
* <p>Only aliases where the given predicate tests successfully are returned. If the indices list contains a non-required reference to
* the index itself - null is returned. Returns <tt>null</tt> if no filtering is required.
*/
Expand Down Expand Up @@ -588,7 +599,7 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
for (int i = 0; i < expressions.size(); i++) {
String expression = expressions.get(i);
if (Strings.isEmpty(expression)) {
throw infe(expression);
throw indexNotFoundException(expression);
}
if (aliasOrIndexExists(options, metaData, expression)) {
if (result != null) {
Expand All @@ -608,8 +619,14 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
result = new HashSet<>(expressions.subList(0, i));
}
if (!Regex.isSimpleMatchPattern(expression)) {
if (!unavailableIgnoredOrExists(options, metaData, expression)) {
throw infe(expression);
//TODO why does wildcard resolver throw exceptions regarding non wildcarded expressions? This should not be done here.
Copy link
Member

@martijnvg martijnvg Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea too...

Copy link
Member Author

@javanna javanna Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to remove this logic but tests start to fail, I will dig deeper another time.

if (options.ignoreUnavailable() == false) {
AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(expression);
if (aliasOrIndex == null) {
throw indexNotFoundException(expression);
} else if (aliasOrIndex.isAlias() && options.ignoreAliases()) {
throw aliasesNotSupportedException(expression);
}
}
if (add) {
result.add(expression);
Expand All @@ -628,7 +645,7 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
result.removeAll(expand);
}
if (options.allowNoIndices() == false && matches.isEmpty()) {
throw infe(expression);
throw indexNotFoundException(expression);
}
if (Regex.isSimpleMatchPattern(expression)) {
wildcardSeen = true;
Expand All @@ -637,17 +654,13 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
return result;
}

private static boolean unavailableIgnoredOrExists(IndicesOptions options, MetaData metaData, String expression) {
return options.ignoreUnavailable() || aliasOrIndexExists(options, metaData, expression);
}

private static boolean aliasOrIndexExists(IndicesOptions options, MetaData metaData, String expression) {
AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(expression);
//treat aliases as unavailable indices when ignoreAliases is set to true (e.g. delete index and update aliases api)
return aliasOrIndex != null && (options.ignoreAliases() == false || aliasOrIndex.isAlias() == false);
}

private static IndexNotFoundException infe(String expression) {
private static IndexNotFoundException indexNotFoundException(String expression) {
IndexNotFoundException infe = new IndexNotFoundException(expression);
infe.setResources("index_or_alias", expression);
return infe;
Expand Down
16 changes: 9 additions & 7 deletions core/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,9 @@
import org.elasticsearch.cluster.metadata.AliasOrIndex;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.StopWatch;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.rest.action.admin.indices.AliasesNotFoundException;
Expand Down Expand Up @@ -442,8 +440,10 @@ public void testDeleteAliases() throws Exception {
assertTrue(admin().indices().prepareAliasesExist("foo").get().exists());
assertFalse(admin().indices().prepareAliasesExist("foo").setIndices("foo_foo").get().exists());
assertTrue(admin().indices().prepareAliasesExist("foo").setIndices("bar_bar").get().exists());
expectThrows(IndexNotFoundException.class, () -> admin().indices().prepareAliases()
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> admin().indices().prepareAliases()
.addAliasAction(AliasActions.remove().index("foo").alias("foo")).execute().actionGet());
assertEquals("The provided expression [foo] matches an alias, specify the corresponding concrete indices instead.",
iae.getMessage());
}

public void testWaitForAliasCreationMultipleShards() throws Exception {
Expand Down Expand Up @@ -824,10 +824,10 @@ public void testAliasesCanBeAddedToIndicesOnly() throws Exception {
logger.info("--> adding [week_20] alias to [2017-05-20]");
assertAcked(admin().indices().prepareAliases().addAlias("2017-05-20", "week_20"));

IndexNotFoundException infe = expectThrows(IndexNotFoundException.class, () -> admin().indices().prepareAliases()
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> admin().indices().prepareAliases()
.addAliasAction(AliasActions.add().index("week_20").alias("tmp")).execute().actionGet());
assertEquals("week_20", infe.getIndex().getName());

assertEquals("The provided expression [week_20] matches an alias, specify the corresponding concrete indices instead.",
iae.getMessage());
assertAcked(admin().indices().prepareAliases().addAliasAction(AliasActions.add().index("2017-05-20").alias("tmp")).execute().get());
}

Expand Down Expand Up @@ -916,8 +916,10 @@ public void testAliasActionRemoveIndex() throws InterruptedException, ExecutionE
assertAcked(admin().indices().prepareAliases().addAlias("foo_foo", "foo"));
assertAcked(admin().indices().prepareAliases().addAlias("bar_bar", "foo"));

expectThrows(IndexNotFoundException.class,
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
() -> client().admin().indices().prepareAliases().removeIndex("foo").execute().actionGet());
assertEquals("The provided expression [foo] matches an alias, specify the corresponding concrete indices instead.",
iae.getMessage());

assertAcked(client().admin().indices().prepareAliases().removeIndex("foo*").execute().get());
assertFalse(client().admin().indices().prepareExists("foo_foo").execute().actionGet().isExists());
Expand Down
38 changes: 28 additions & 10 deletions core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,10 @@ public void testConcreteIndicesWildcardAndAliases() {
assertEquals(1, indexNamesIndexWildcard.length);
assertEquals("foo_foo", indexNamesIndexWildcard[0]);

IndexNotFoundException infe = expectThrows(IndexNotFoundException.class,
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, ignoreAliasesOptions, "foo"));
assertThat(infe.getIndex().getName(), equalTo("foo"));
assertEquals("The provided expression [foo] matches an alias, specify the corresponding concrete indices instead.",
iae.getMessage());

// when ignoreAliases option is not set, concreteIndexNames resolves the provided
// expressions against the defined indices and aliases
Expand Down Expand Up @@ -1005,11 +1006,13 @@ public void testDeleteIndexIgnoresAliases() {
IndexNotFoundException infe = expectThrows(IndexNotFoundException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, new DeleteIndexRequest("does_not_exist")));
assertEquals("does_not_exist", infe.getIndex().getName());
assertEquals("no such index", infe.getMessage());
}
{
IndexNotFoundException infe = expectThrows(IndexNotFoundException.class,
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, new DeleteIndexRequest("test-alias")));
assertEquals("test-alias", infe.getIndex().getName());
assertEquals("The provided expression [test-alias] matches an alias, " +
"specify the corresponding concrete indices instead.", iae.getMessage());
}
{
DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest("test-alias");
Expand Down Expand Up @@ -1049,11 +1052,16 @@ public void testIndicesAliasesRequestIgnoresAliases() {
ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build();
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.add().index("test-alias");
expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
assertEquals("The provided expression [test-alias] matches an alias, " +
"specify the corresponding concrete indices instead.", iae.getMessage());
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.add().index("test-a*");
expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
IndexNotFoundException infe = expectThrows(IndexNotFoundException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
assertEquals("test-a*", infe.getIndex().getName());
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.add().index("test-index");
Expand All @@ -1069,11 +1077,16 @@ public void testIndicesAliasesRequestIgnoresAliases() {
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.remove().index("test-alias");
expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
assertEquals("The provided expression [test-alias] matches an alias, " +
"specify the corresponding concrete indices instead.", iae.getMessage());
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.remove().index("test-a*");
expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
IndexNotFoundException infe = expectThrows(IndexNotFoundException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
assertEquals("test-a*", infe.getIndex().getName());
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.remove().index("test-index");
Expand All @@ -1089,11 +1102,16 @@ public void testIndicesAliasesRequestIgnoresAliases() {
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.removeIndex().index("test-alias");
expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
assertEquals("The provided expression [test-alias] matches an alias, " +
"specify the corresponding concrete indices instead.", iae.getMessage());
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.removeIndex().index("test-a*");
expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
IndexNotFoundException infe = expectThrows(IndexNotFoundException.class,
() -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions));
assertEquals("test-a*", infe.getIndex().getName());
}
{
IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.removeIndex().index("test-index");
Expand Down
Loading