Preserve index_uuid when creating QueryShardException (#32677)
As part of #32608 we made sure that the fully qualified index name is taken from the query shard context whenever creating a new `QueryShardException`. That change introduced a regression as instead of setting the entire `Index` object to the exception, which holds index name and index uuid, we ended up setting only the index name (including cluster alias). With this commit we make sure that the index uuid does not get lost and we try to lower the chances that a similar bug makes it in another time. That's done by making `QueryShardContext` return the fully qualified `Index` (which also holds the uuid) rather than only the fully qualified index name.
This commit is contained in:
parent
4cbcc1d659
commit
5c2ef5e869
|
@ -756,7 +756,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
|
|||
@Override
|
||||
@SuppressWarnings("unchecked")
|
||||
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
|
||||
IndexFieldData.Builder builder = fieldType.fielddataBuilder(shardContext.getFullyQualifiedIndexName());
|
||||
IndexFieldData.Builder builder = fieldType.fielddataBuilder(shardContext.getFullyQualifiedIndex().getName());
|
||||
IndexFieldDataCache cache = new IndexFieldDataCache.None();
|
||||
CircuitBreakerService circuitBreaker = new NoneCircuitBreakerService();
|
||||
return (IFD) builder.build(shardContext.getIndexSettings(), fieldType, cache, circuitBreaker,
|
||||
|
@ -764,5 +764,4 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
|
|||
}
|
||||
};
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -126,7 +126,7 @@ public class IndexFieldMapper extends MetadataFieldMapper {
|
|||
*/
|
||||
@Override
|
||||
public Query termQuery(Object value, @Nullable QueryShardContext context) {
|
||||
if (isSameIndex(value, context.getFullyQualifiedIndexName())) {
|
||||
if (isSameIndex(value, context.getFullyQualifiedIndex().getName())) {
|
||||
return Queries.newMatchAllQuery();
|
||||
} else {
|
||||
return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.index().getName() + " vs. " + value);
|
||||
|
@ -139,14 +139,14 @@ public class IndexFieldMapper extends MetadataFieldMapper {
|
|||
return super.termsQuery(values, context);
|
||||
}
|
||||
for (Object value : values) {
|
||||
if (isSameIndex(value, context.getFullyQualifiedIndexName())) {
|
||||
if (isSameIndex(value, context.getFullyQualifiedIndex().getName())) {
|
||||
// No need to OR these clauses - we can only logically be
|
||||
// running in the context of just one of these index names.
|
||||
return Queries.newMatchAllQuery();
|
||||
}
|
||||
}
|
||||
// None of the listed index names are this one
|
||||
return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.getFullyQualifiedIndexName()
|
||||
return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.getFullyQualifiedIndex().getName()
|
||||
+ " vs. " + values);
|
||||
}
|
||||
|
||||
|
@ -189,5 +189,4 @@ public class IndexFieldMapper extends MetadataFieldMapper {
|
|||
protected void doMerge(Mapper mergeWith) {
|
||||
// nothing to do
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -83,7 +83,7 @@ public class QueryShardContext extends QueryRewriteContext {
|
|||
private String[] types = Strings.EMPTY_ARRAY;
|
||||
private boolean cachable = true;
|
||||
private final SetOnce<Boolean> frozen = new SetOnce<>();
|
||||
private final String fullyQualifiedIndexName;
|
||||
private final Index fullyQualifiedIndex;
|
||||
|
||||
public void setTypes(String... types) {
|
||||
this.types = types;
|
||||
|
@ -116,7 +116,8 @@ public class QueryShardContext extends QueryRewriteContext {
|
|||
this.indexSettings = indexSettings;
|
||||
this.reader = reader;
|
||||
this.clusterAlias = clusterAlias;
|
||||
this.fullyQualifiedIndexName = RemoteClusterAware.buildRemoteIndexName(clusterAlias, indexSettings.getIndex().getName());
|
||||
this.fullyQualifiedIndex = new Index(RemoteClusterAware.buildRemoteIndexName(clusterAlias, indexSettings.getIndex().getName()),
|
||||
indexSettings.getIndex().getUUID());
|
||||
}
|
||||
|
||||
public QueryShardContext(QueryShardContext source) {
|
||||
|
@ -163,7 +164,7 @@ public class QueryShardContext extends QueryRewriteContext {
|
|||
}
|
||||
|
||||
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
|
||||
return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndexName);
|
||||
return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName());
|
||||
}
|
||||
|
||||
public void addNamedQuery(String name, Query query) {
|
||||
|
@ -275,7 +276,7 @@ public class QueryShardContext extends QueryRewriteContext {
|
|||
public SearchLookup lookup() {
|
||||
if (lookup == null) {
|
||||
lookup = new SearchLookup(getMapperService(),
|
||||
mappedFieldType -> indexFieldDataService.apply(mappedFieldType, fullyQualifiedIndexName), types);
|
||||
mappedFieldType -> indexFieldDataService.apply(mappedFieldType, fullyQualifiedIndex.getName()), types);
|
||||
}
|
||||
return lookup;
|
||||
}
|
||||
|
@ -426,9 +427,9 @@ public class QueryShardContext extends QueryRewriteContext {
|
|||
}
|
||||
|
||||
/**
|
||||
* Returns the fully qualified index name including a remote cluster alias if applicable
|
||||
* Returns the fully qualified index including a remote cluster alias if applicable, and the index uuid
|
||||
*/
|
||||
public String getFullyQualifiedIndexName() {
|
||||
return fullyQualifiedIndexName;
|
||||
public Index getFullyQualifiedIndex() {
|
||||
return fullyQualifiedIndex;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -37,16 +37,15 @@ public class QueryShardException extends ElasticsearchException {
|
|||
}
|
||||
|
||||
public QueryShardException(QueryShardContext context, String msg, Throwable cause, Object... args) {
|
||||
super(msg, cause, args);
|
||||
setIndex(context.getFullyQualifiedIndexName());
|
||||
this(context.getFullyQualifiedIndex(), msg, cause, args);
|
||||
}
|
||||
|
||||
/**
|
||||
* This constructor is provided for use in unit tests where a
|
||||
* {@link QueryShardContext} may not be available
|
||||
*/
|
||||
public QueryShardException(Index index, String msg, Throwable cause) {
|
||||
super(msg, cause);
|
||||
public QueryShardException(Index index, String msg, Throwable cause, Object... args) {
|
||||
super(msg, cause, args);
|
||||
setIndex(index);
|
||||
}
|
||||
|
||||
|
|
|
@ -22,8 +22,10 @@ import org.apache.lucene.search.MatchNoDocsQuery;
|
|||
import org.apache.lucene.search.Query;
|
||||
import org.elasticsearch.Version;
|
||||
import org.elasticsearch.cluster.metadata.IndexMetaData;
|
||||
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
|
||||
import org.elasticsearch.common.lucene.search.Queries;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
|
||||
import org.elasticsearch.index.IndexSettings;
|
||||
import org.elasticsearch.index.fielddata.IndexFieldData;
|
||||
import org.elasticsearch.index.fielddata.plain.AbstractAtomicOrdinalsFieldData;
|
||||
|
@ -37,6 +39,7 @@ import org.elasticsearch.test.ESTestCase;
|
|||
import org.hamcrest.Matchers;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Collections;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
|
@ -49,24 +52,7 @@ import static org.mockito.Mockito.when;
|
|||
public class QueryShardContextTests extends ESTestCase {
|
||||
|
||||
public void testFailIfFieldMappingNotFound() {
|
||||
IndexMetaData.Builder indexMetadataBuilder = new IndexMetaData.Builder("index");
|
||||
indexMetadataBuilder.settings(Settings.builder().put("index.version.created", Version.CURRENT)
|
||||
.put("index.number_of_shards", 1)
|
||||
.put("index.number_of_replicas", 1)
|
||||
);
|
||||
IndexMetaData indexMetaData = indexMetadataBuilder.build();
|
||||
IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
|
||||
MapperService mapperService = mock(MapperService.class);
|
||||
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
|
||||
when(mapperService.index()).thenReturn(indexMetaData.getIndex());
|
||||
final long nowInMillis = randomNonNegativeLong();
|
||||
|
||||
QueryShardContext context = new QueryShardContext(
|
||||
0, indexSettings, null, (mappedFieldType, idxName) ->
|
||||
mappedFieldType.fielddataBuilder(idxName).build(indexSettings, mappedFieldType, null, null, null)
|
||||
, mapperService, null, null, xContentRegistry(), writableRegistry(), null, null,
|
||||
() -> nowInMillis, null);
|
||||
|
||||
QueryShardContext context = createQueryShardContext(IndexMetaData.INDEX_UUID_NA_VALUE, null);
|
||||
context.setAllowUnmappedFields(false);
|
||||
MappedFieldType fieldType = new TextFieldMapper.TextFieldType();
|
||||
MappedFieldType result = context.failIfFieldMappingNotFound("name", fieldType);
|
||||
|
@ -91,30 +77,16 @@ public class QueryShardContextTests extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testClusterAlias() throws IOException {
|
||||
IndexMetaData.Builder indexMetadataBuilder = new IndexMetaData.Builder("index");
|
||||
indexMetadataBuilder.settings(Settings.builder().put("index.version.created", Version.CURRENT)
|
||||
.put("index.number_of_shards", 1)
|
||||
.put("index.number_of_replicas", 1)
|
||||
);
|
||||
IndexMetaData indexMetaData = indexMetadataBuilder.build();
|
||||
IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
|
||||
MapperService mapperService = mock(MapperService.class);
|
||||
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
|
||||
when(mapperService.index()).thenReturn(indexMetaData.getIndex());
|
||||
final long nowInMillis = randomNonNegativeLong();
|
||||
|
||||
Mapper.BuilderContext ctx = new Mapper.BuilderContext(indexSettings.getSettings(), new ContentPath());
|
||||
IndexFieldMapper mapper = new IndexFieldMapper.Builder(null).build(ctx);
|
||||
final String clusterAlias = randomBoolean() ? null : "remote_cluster";
|
||||
QueryShardContext context = new QueryShardContext(
|
||||
0, indexSettings, null, (mappedFieldType, indexname) ->
|
||||
mappedFieldType.fielddataBuilder(indexname).build(indexSettings, mappedFieldType, null, null, mapperService)
|
||||
, mapperService, null, null, xContentRegistry(), writableRegistry(), null, null,
|
||||
() -> nowInMillis, clusterAlias);
|
||||
QueryShardContext context = createQueryShardContext(IndexMetaData.INDEX_UUID_NA_VALUE, clusterAlias);
|
||||
|
||||
|
||||
Mapper.BuilderContext ctx = new Mapper.BuilderContext(context.getIndexSettings().getSettings(), new ContentPath());
|
||||
IndexFieldMapper mapper = new IndexFieldMapper.Builder(null).build(ctx);
|
||||
|
||||
IndexFieldData<?> forField = context.getForField(mapper.fieldType());
|
||||
String expected = clusterAlias == null ? indexMetaData.getIndex().getName()
|
||||
: clusterAlias + ":" + indexMetaData.getIndex().getName();
|
||||
String expected = clusterAlias == null ? context.getIndexSettings().getIndexMetaData().getIndex().getName()
|
||||
: clusterAlias + ":" + context.getIndexSettings().getIndex().getName();
|
||||
assertEquals(expected, ((AbstractAtomicOrdinalsFieldData)forField.load(null)).getOrdinalsValues().lookupOrd(0).utf8ToString());
|
||||
Query query = mapper.fieldType().termQuery("index", context);
|
||||
if (clusterAlias == null) {
|
||||
|
@ -133,4 +105,32 @@ public class QueryShardContextTests extends ESTestCase {
|
|||
assertThat(query, Matchers.instanceOf(MatchNoDocsQuery.class));
|
||||
}
|
||||
|
||||
public void testGetFullyQualifiedIndex() {
|
||||
String clusterAlias = randomAlphaOfLengthBetween(5, 10);
|
||||
String indexUuid = randomAlphaOfLengthBetween(3, 10);
|
||||
QueryShardContext shardContext = createQueryShardContext(indexUuid, clusterAlias);
|
||||
assertThat(shardContext.getFullyQualifiedIndex().getName(), equalTo(clusterAlias + ":index"));
|
||||
assertThat(shardContext.getFullyQualifiedIndex().getUUID(), equalTo(indexUuid));
|
||||
}
|
||||
|
||||
public static QueryShardContext createQueryShardContext(String indexUuid, String clusterAlias) {
|
||||
IndexMetaData.Builder indexMetadataBuilder = new IndexMetaData.Builder("index");
|
||||
indexMetadataBuilder.settings(Settings.builder().put("index.version.created", Version.CURRENT)
|
||||
.put("index.number_of_shards", 1)
|
||||
.put("index.number_of_replicas", 1)
|
||||
.put(IndexMetaData.SETTING_INDEX_UUID, indexUuid)
|
||||
);
|
||||
IndexMetaData indexMetaData = indexMetadataBuilder.build();
|
||||
IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
|
||||
MapperService mapperService = mock(MapperService.class);
|
||||
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
|
||||
when(mapperService.index()).thenReturn(indexMetaData.getIndex());
|
||||
final long nowInMillis = randomNonNegativeLong();
|
||||
|
||||
return new QueryShardContext(
|
||||
0, indexSettings, null, (mappedFieldType, idxName) ->
|
||||
mappedFieldType.fielddataBuilder(idxName).build(indexSettings, mappedFieldType, null, null, null)
|
||||
, mapperService, null, null, NamedXContentRegistry.EMPTY, new NamedWriteableRegistry(Collections.emptyList()), null, null,
|
||||
() -> nowInMillis, clusterAlias);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,53 @@
|
|||
/*
|
||||
* Licensed to Elasticsearch under one or more contributor
|
||||
* license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright
|
||||
* ownership. Elasticsearch licenses this file to you under
|
||||
* the Apache License, Version 2.0 (the "License"); you may
|
||||
* not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
package org.elasticsearch.index.query;
|
||||
|
||||
import org.elasticsearch.index.Index;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
|
||||
public class QueryShardExceptionTests extends ESTestCase {
|
||||
|
||||
public void testCreateFromQueryShardContext() {
|
||||
String indexUuid = randomAlphaOfLengthBetween(5, 10);
|
||||
String clusterAlias = randomAlphaOfLengthBetween(5, 10);
|
||||
QueryShardContext queryShardContext = QueryShardContextTests.createQueryShardContext(indexUuid, clusterAlias);
|
||||
{
|
||||
QueryShardException queryShardException = new QueryShardException(queryShardContext, "error");
|
||||
assertThat(queryShardException.getIndex().getName(), equalTo(clusterAlias + ":index"));
|
||||
assertThat(queryShardException.getIndex().getUUID(), equalTo(indexUuid));
|
||||
}
|
||||
{
|
||||
QueryShardException queryShardException = new QueryShardException(queryShardContext, "error", new IllegalArgumentException());
|
||||
assertThat(queryShardException.getIndex().getName(), equalTo(clusterAlias + ":index"));
|
||||
assertThat(queryShardException.getIndex().getUUID(), equalTo(indexUuid));
|
||||
}
|
||||
}
|
||||
|
||||
public void testCreateFromIndex() {
|
||||
String indexUuid = randomAlphaOfLengthBetween(5, 10);
|
||||
String indexName = randomAlphaOfLengthBetween(5, 10);
|
||||
Index index = new Index(indexName, indexUuid);
|
||||
QueryShardException queryShardException = new QueryShardException(index, "error", new IllegalArgumentException());
|
||||
assertThat(queryShardException.getIndex().getName(), equalTo(indexName));
|
||||
assertThat(queryShardException.getIndex().getUUID(), equalTo(indexUuid));
|
||||
}
|
||||
}
|
|
@ -32,7 +32,6 @@ import java.io.IOException;
|
|||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.elasticsearch.test.AbstractBuilderTestCase.STRING_ALIAS_FIELD_NAME;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
|
||||
|
@ -143,7 +142,7 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase<WildcardQue
|
|||
|
||||
public void testIndexWildcard() throws IOException {
|
||||
QueryShardContext context = createShardContext();
|
||||
String index = context.getFullyQualifiedIndexName();
|
||||
String index = context.getFullyQualifiedIndex().getName();
|
||||
|
||||
Query query = new WildcardQueryBuilder("_index", index).doToQuery(context);
|
||||
assertThat(query instanceof MatchAllDocsQuery, equalTo(true));
|
||||
|
|
Loading…
Reference in New Issue