Skip to content

Commit 95290d6

Browse files
committed
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.
1 parent bec4af5 commit 95290d6

File tree

7 files changed

+107
-56
lines changed

7 files changed

+107
-56
lines changed

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -772,13 +772,12 @@ public BitSetProducer bitsetFilter(Query query) {
772772
@Override
773773
@SuppressWarnings("unchecked")
774774
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
775-
IndexFieldData.Builder builder = fieldType.fielddataBuilder(shardContext.getFullyQualifiedIndexName());
775+
IndexFieldData.Builder builder = fieldType.fielddataBuilder(shardContext.getFullyQualifiedIndex().getName());
776776
IndexFieldDataCache cache = new IndexFieldDataCache.None();
777777
CircuitBreakerService circuitBreaker = new NoneCircuitBreakerService();
778778
return (IFD) builder.build(shardContext.getIndexSettings(), fieldType, cache, circuitBreaker,
779779
shardContext.getMapperService());
780780
}
781781
};
782782
}
783-
784783
}

server/src/main/java/org/elasticsearch/index/mapper/IndexFieldMapper.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public Query existsQuery(QueryShardContext context) {
130130
*/
131131
@Override
132132
public Query termQuery(Object value, @Nullable QueryShardContext context) {
133-
if (isSameIndex(value, context.getFullyQualifiedIndexName())) {
133+
if (isSameIndex(value, context.getFullyQualifiedIndex().getName())) {
134134
return Queries.newMatchAllQuery();
135135
} else {
136136
return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.index().getName() + " vs. " + value);
@@ -143,14 +143,14 @@ public Query termsQuery(List values, QueryShardContext context) {
143143
return super.termsQuery(values, context);
144144
}
145145
for (Object value : values) {
146-
if (isSameIndex(value, context.getFullyQualifiedIndexName())) {
146+
if (isSameIndex(value, context.getFullyQualifiedIndex().getName())) {
147147
// No need to OR these clauses - we can only logically be
148148
// running in the context of just one of these index names.
149149
return Queries.newMatchAllQuery();
150150
}
151151
}
152152
// None of the listed index names are this one
153-
return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.getFullyQualifiedIndexName()
153+
return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.getFullyQualifiedIndex().getName()
154154
+ " vs. " + values);
155155
}
156156

@@ -193,5 +193,4 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
193193
protected void doMerge(Mapper mergeWith, boolean updateAllTypes) {
194194
// nothing to do
195195
}
196-
197196
}

server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

+8-7
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public class QueryShardContext extends QueryRewriteContext {
8282
private String[] types = Strings.EMPTY_ARRAY;
8383
private boolean cachable = true;
8484
private final SetOnce<Boolean> frozen = new SetOnce<>();
85-
private final String fullyQualifiedIndexName;
85+
private final Index fullyQualifiedIndex;
8686

8787
public void setTypes(String... types) {
8888
this.types = types;
@@ -115,7 +115,8 @@ public QueryShardContext(int shardId, IndexSettings indexSettings, BitsetFilterC
115115
this.indexSettings = indexSettings;
116116
this.reader = reader;
117117
this.clusterAlias = clusterAlias;
118-
this.fullyQualifiedIndexName = RemoteClusterAware.buildRemoteIndexName(clusterAlias, indexSettings.getIndex().getName());
118+
this.fullyQualifiedIndex = new Index(RemoteClusterAware.buildRemoteIndexName(clusterAlias, indexSettings.getIndex().getName()),
119+
indexSettings.getIndex().getUUID());
119120
}
120121

121122
public QueryShardContext(QueryShardContext source) {
@@ -162,7 +163,7 @@ public BitSetProducer bitsetFilter(Query filter) {
162163
}
163164

164165
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
165-
return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndexName);
166+
return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName());
166167
}
167168

168169
public void addNamedQuery(String name, Query query) {
@@ -276,7 +277,7 @@ public Collection<String> queryTypes() {
276277
public SearchLookup lookup() {
277278
if (lookup == null) {
278279
lookup = new SearchLookup(getMapperService(),
279-
mappedFieldType -> indexFieldDataService.apply(mappedFieldType, fullyQualifiedIndexName), types);
280+
mappedFieldType -> indexFieldDataService.apply(mappedFieldType, fullyQualifiedIndex.getName()), types);
280281
}
281282
return lookup;
282283
}
@@ -427,9 +428,9 @@ public IndexReader getIndexReader() {
427428
}
428429

429430
/**
430-
* Returns the fully qualified index name including a remote cluster alias if applicable
431+
* Returns the fully qualified index including a remote cluster alias if applicable, and the index uuid
431432
*/
432-
public String getFullyQualifiedIndexName() {
433-
return fullyQualifiedIndexName;
433+
public Index getFullyQualifiedIndex() {
434+
return fullyQualifiedIndex;
434435
}
435436
}

server/src/main/java/org/elasticsearch/index/query/QueryShardException.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,15 @@ public QueryShardException(QueryShardContext context, String msg, Object... args
3737
}
3838

3939
public QueryShardException(QueryShardContext context, String msg, Throwable cause, Object... args) {
40-
super(msg, cause, args);
41-
setIndex(context.getFullyQualifiedIndexName());
40+
this(context.getFullyQualifiedIndex(), msg, cause, args);
4241
}
4342

4443
/**
4544
* This constructor is provided for use in unit tests where a
4645
* {@link QueryShardContext} may not be available
4746
*/
48-
public QueryShardException(Index index, String msg, Throwable cause) {
49-
super(msg, cause);
47+
public QueryShardException(Index index, String msg, Throwable cause, Object... args) {
48+
super(msg, cause, args);
5049
setIndex(index);
5150
}
5251

server/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java

+38-38
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
import org.apache.lucene.search.Query;
2323
import org.elasticsearch.Version;
2424
import org.elasticsearch.cluster.metadata.IndexMetaData;
25+
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
2526
import org.elasticsearch.common.lucene.search.Queries;
2627
import org.elasticsearch.common.settings.Settings;
28+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2729
import org.elasticsearch.index.IndexSettings;
2830
import org.elasticsearch.index.fielddata.IndexFieldData;
2931
import org.elasticsearch.index.fielddata.plain.AbstractAtomicOrdinalsFieldData;
@@ -37,6 +39,7 @@
3739
import org.hamcrest.Matchers;
3840

3941
import java.io.IOException;
42+
import java.util.Collections;
4043

4144
import static org.hamcrest.Matchers.equalTo;
4245
import static org.hamcrest.Matchers.instanceOf;
@@ -49,24 +52,7 @@
4952
public class QueryShardContextTests extends ESTestCase {
5053

5154
public void testFailIfFieldMappingNotFound() {
52-
IndexMetaData.Builder indexMetadataBuilder = new IndexMetaData.Builder("index");
53-
indexMetadataBuilder.settings(Settings.builder().put("index.version.created", Version.CURRENT)
54-
.put("index.number_of_shards", 1)
55-
.put("index.number_of_replicas", 1)
56-
);
57-
IndexMetaData indexMetaData = indexMetadataBuilder.build();
58-
IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
59-
MapperService mapperService = mock(MapperService.class);
60-
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
61-
when(mapperService.index()).thenReturn(indexMetaData.getIndex());
62-
final long nowInMillis = randomNonNegativeLong();
63-
64-
QueryShardContext context = new QueryShardContext(
65-
0, indexSettings, null, (mappedFieldType, idxName) ->
66-
mappedFieldType.fielddataBuilder(idxName).build(indexSettings, mappedFieldType, null, null, null)
67-
, mapperService, null, null, xContentRegistry(), writableRegistry(), null, null,
68-
() -> nowInMillis, null);
69-
55+
QueryShardContext context = createQueryShardContext(IndexMetaData.INDEX_UUID_NA_VALUE, null);
7056
context.setAllowUnmappedFields(false);
7157
MappedFieldType fieldType = new TextFieldMapper.TextFieldType();
7258
MappedFieldType result = context.failIfFieldMappingNotFound("name", fieldType);
@@ -91,30 +77,16 @@ public void testFailIfFieldMappingNotFound() {
9177
}
9278

9379
public void testClusterAlias() throws IOException {
94-
IndexMetaData.Builder indexMetadataBuilder = new IndexMetaData.Builder("index");
95-
indexMetadataBuilder.settings(Settings.builder().put("index.version.created", Version.CURRENT)
96-
.put("index.number_of_shards", 1)
97-
.put("index.number_of_replicas", 1)
98-
);
99-
IndexMetaData indexMetaData = indexMetadataBuilder.build();
100-
IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
101-
MapperService mapperService = mock(MapperService.class);
102-
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
103-
when(mapperService.index()).thenReturn(indexMetaData.getIndex());
104-
final long nowInMillis = randomNonNegativeLong();
80+
final String clusterAlias = randomBoolean() ? null : "remote_cluster";
81+
QueryShardContext context = createQueryShardContext(IndexMetaData.INDEX_UUID_NA_VALUE, clusterAlias);
10582

106-
Mapper.BuilderContext ctx = new Mapper.BuilderContext(indexSettings.getSettings(), new ContentPath());
83+
84+
Mapper.BuilderContext ctx = new Mapper.BuilderContext(context.getIndexSettings().getSettings(), new ContentPath());
10785
IndexFieldMapper mapper = new IndexFieldMapper.Builder(null).build(ctx);
108-
final String clusterAlias = randomBoolean() ? null : "remote_cluster";
109-
QueryShardContext context = new QueryShardContext(
110-
0, indexSettings, null, (mappedFieldType, indexname) ->
111-
mappedFieldType.fielddataBuilder(indexname).build(indexSettings, mappedFieldType, null, null, mapperService)
112-
, mapperService, null, null, xContentRegistry(), writableRegistry(), null, null,
113-
() -> nowInMillis, clusterAlias);
11486

11587
IndexFieldData<?> forField = context.getForField(mapper.fieldType());
116-
String expected = clusterAlias == null ? indexMetaData.getIndex().getName()
117-
: clusterAlias + ":" + indexMetaData.getIndex().getName();
88+
String expected = clusterAlias == null ? context.getIndexSettings().getIndexMetaData().getIndex().getName()
89+
: clusterAlias + ":" + context.getIndexSettings().getIndex().getName();
11890
assertEquals(expected, ((AbstractAtomicOrdinalsFieldData)forField.load(null)).getOrdinalsValues().lookupOrd(0).utf8ToString());
11991
Query query = mapper.fieldType().termQuery("index", context);
12092
if (clusterAlias == null) {
@@ -133,4 +105,32 @@ public void testClusterAlias() throws IOException {
133105
assertThat(query, Matchers.instanceOf(MatchNoDocsQuery.class));
134106
}
135107

108+
public void testGetFullyQualifiedIndex() {
109+
String clusterAlias = randomAlphaOfLengthBetween(5, 10);
110+
String indexUuid = randomAlphaOfLengthBetween(3, 10);
111+
QueryShardContext shardContext = createQueryShardContext(indexUuid, clusterAlias);
112+
assertThat(shardContext.getFullyQualifiedIndex().getName(), equalTo(clusterAlias + ":index"));
113+
assertThat(shardContext.getFullyQualifiedIndex().getUUID(), equalTo(indexUuid));
114+
}
115+
116+
public static QueryShardContext createQueryShardContext(String indexUuid, String clusterAlias) {
117+
IndexMetaData.Builder indexMetadataBuilder = new IndexMetaData.Builder("index");
118+
indexMetadataBuilder.settings(Settings.builder().put("index.version.created", Version.CURRENT)
119+
.put("index.number_of_shards", 1)
120+
.put("index.number_of_replicas", 1)
121+
.put(IndexMetaData.SETTING_INDEX_UUID, indexUuid)
122+
);
123+
IndexMetaData indexMetaData = indexMetadataBuilder.build();
124+
IndexSettings indexSettings = new IndexSettings(indexMetaData, Settings.EMPTY);
125+
MapperService mapperService = mock(MapperService.class);
126+
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
127+
when(mapperService.index()).thenReturn(indexMetaData.getIndex());
128+
final long nowInMillis = randomNonNegativeLong();
129+
130+
return new QueryShardContext(
131+
0, indexSettings, null, (mappedFieldType, idxName) ->
132+
mappedFieldType.fielddataBuilder(idxName).build(indexSettings, mappedFieldType, null, null, null)
133+
, mapperService, null, null, NamedXContentRegistry.EMPTY, new NamedWriteableRegistry(Collections.emptyList()), null, null,
134+
() -> nowInMillis, clusterAlias);
135+
}
136136
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.index.query;
21+
22+
import org.elasticsearch.index.Index;
23+
import org.elasticsearch.test.ESTestCase;
24+
25+
import static org.hamcrest.CoreMatchers.equalTo;
26+
27+
public class QueryShardExceptionTests extends ESTestCase {
28+
29+
public void testCreateFromQueryShardContext() {
30+
String indexUuid = randomAlphaOfLengthBetween(5, 10);
31+
String clusterAlias = randomAlphaOfLengthBetween(5, 10);
32+
QueryShardContext queryShardContext = QueryShardContextTests.createQueryShardContext(indexUuid, clusterAlias);
33+
{
34+
QueryShardException queryShardException = new QueryShardException(queryShardContext, "error");
35+
assertThat(queryShardException.getIndex().getName(), equalTo(clusterAlias + ":index"));
36+
assertThat(queryShardException.getIndex().getUUID(), equalTo(indexUuid));
37+
}
38+
{
39+
QueryShardException queryShardException = new QueryShardException(queryShardContext, "error", new IllegalArgumentException());
40+
assertThat(queryShardException.getIndex().getName(), equalTo(clusterAlias + ":index"));
41+
assertThat(queryShardException.getIndex().getUUID(), equalTo(indexUuid));
42+
}
43+
}
44+
45+
public void testCreateFromIndex() {
46+
String indexUuid = randomAlphaOfLengthBetween(5, 10);
47+
String indexName = randomAlphaOfLengthBetween(5, 10);
48+
Index index = new Index(indexName, indexUuid);
49+
QueryShardException queryShardException = new QueryShardException(index, "error", new IllegalArgumentException());
50+
assertThat(queryShardException.getIndex().getName(), equalTo(indexName));
51+
assertThat(queryShardException.getIndex().getUUID(), equalTo(indexUuid));
52+
}
53+
}

server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public void testIndexWildcard() throws IOException {
144144
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
145145

146146
QueryShardContext context = createShardContext();
147-
String index = context.getFullyQualifiedIndexName();
147+
String index = context.getFullyQualifiedIndex().getName();
148148

149149
Query query = new WildcardQueryBuilder("_index", index).doToQuery(context);
150150
assertThat(query instanceof MatchAllDocsQuery, equalTo(true));

0 commit comments

Comments
 (0)