Remove `_index` fielddata hack if cluster alias is present (#26082)

We introduced a hack in #25885 to respect the cluster alias if available on the `_index` field. This is important if aggregations or other field data related operations are executed. Yet, we added a small hack that duplicated an implementation detail from the `_index` field data builder to make this work. This change adds a necessary but simple API change that allows us to remove the hack and only have a single implementation.
This commit is contained in:
Simon Willnauer 2017-08-08 09:24:24 +02:00 committed by GitHub
parent f0cba4fce5
commit 82fa531ab4
29 changed files with 56 additions and 57 deletions

View File

@ -105,10 +105,14 @@ public class IndexFieldDataService extends AbstractIndexComponent implements Clo
ExceptionsHelper.maybeThrowRuntimeAndSuppress(exceptions);
}
@SuppressWarnings("unchecked")
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
return getForField(fieldType, index().getName());
}
@SuppressWarnings("unchecked")
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType, String fullyQualifiedIndexName) {
final String fieldName = fieldType.name();
IndexFieldData.Builder builder = fieldType.fielddataBuilder();
IndexFieldData.Builder builder = fieldType.fielddataBuilder(fullyQualifiedIndexName);
IndexFieldDataCache cache;
synchronized (this) {

View File

@ -121,7 +121,7 @@ public class BinaryFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new BytesBinaryDVIndexFieldData.Builder();
}

View File

@ -182,7 +182,7 @@ public class BooleanFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder().numericType(NumericType.BOOLEAN);
}

View File

@ -356,7 +356,7 @@ public class DateFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder().numericType(NumericType.DATE);
}

View File

@ -175,7 +175,7 @@ public class GeoPointFieldMapper extends FieldMapper implements ArrayValueMapper
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new AbstractLatLonPointDVIndexFieldData.Builder();
}

View File

@ -153,7 +153,7 @@ public class IdFieldMapper extends MetadataFieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
if (indexOptions() == IndexOptions.NONE) {
throw new IllegalArgumentException("Fielddata access on the _uid field is disallowed");
}

View File

@ -157,8 +157,8 @@ public class IndexFieldMapper extends MetadataFieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
return new ConstantIndexFieldData.Builder(mapperService -> mapperService.index().getName());
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new ConstantIndexFieldData.Builder(mapperService -> fullyQualifiedIndexName);
}
}

View File

@ -281,7 +281,7 @@ public class IpFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder().scriptFunction(IpScriptDocValues::new);
}

View File

@ -219,7 +219,7 @@ public final class KeywordFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder();
}

View File

@ -98,8 +98,10 @@ public abstract class MappedFieldType extends FieldType {
* @throws IllegalArgumentException if the fielddata is not supported on this type.
* An IllegalArgumentException is needed in order to return an http error 400
* when this error occurs in a request. see: {@link org.elasticsearch.ExceptionsHelper#status}
**/
public IndexFieldData.Builder fielddataBuilder() {
*
* @param fullyQualifiedIndexName the name of the index this field-data is build for
* */
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
throw new IllegalArgumentException("Fielddata is not supported on field [" + name() + "] of type [" + typeName() + "]");
}
@ -322,7 +324,7 @@ public abstract class MappedFieldType extends FieldType {
*/
public boolean isAggregatable() {
try {
fielddataBuilder();
fielddataBuilder("");
return true;
} catch (IllegalArgumentException e) {
return false;

View File

@ -855,7 +855,7 @@ public class NumberFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder().numericType(type.numericType());
}

View File

@ -41,7 +41,6 @@ import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
@ -198,7 +197,7 @@ public class ParentFieldMapper extends MetadataFieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder();
}
}

View File

@ -259,7 +259,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new IndexFieldData.Builder() {
@Override

View File

@ -204,7 +204,7 @@ public class SeqNoFieldMapper extends MetadataFieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder().numericType(NumericType.LONG);
}

View File

@ -283,7 +283,7 @@ public class TextFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
if (fielddata == false) {
throw new IllegalArgumentException("Fielddata is disabled on text fields by default. Set fielddata=true on [" + name()
+ "] in order to load fielddata in memory by uninverting the inverted index. Note that this can however "

View File

@ -109,7 +109,7 @@ public class TypeFieldMapper extends MetadataFieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
if (hasDocValues()) {
return new DocValuesIndexFieldData.Builder();
} else {

View File

@ -110,7 +110,7 @@ public class UidFieldMapper extends MetadataFieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
if (indexOptions() == IndexOptions.NONE) {
DEPRECATION_LOGGER.deprecated("Fielddata access on the _uid field is deprecated, use _id instead");
return new IndexFieldData.Builder() {
@ -118,7 +118,7 @@ public class UidFieldMapper extends MetadataFieldMapper {
public IndexFieldData<?> build(IndexSettings indexSettings, MappedFieldType fieldType, IndexFieldDataCache cache,
CircuitBreakerService breakerService, MapperService mapperService) {
MappedFieldType idFieldType = mapperService.fullName(IdFieldMapper.NAME);
IndexFieldData<?> idFieldData = idFieldType.fielddataBuilder()
IndexFieldData<?> idFieldData = idFieldType.fielddataBuilder(fullyQualifiedIndexName)
.build(indexSettings, idFieldType, cache, breakerService, mapperService);
final String type = mapperService.types().iterator().next();
return new UidIndexFieldData(indexSettings.getIndex(), type, idFieldData);

View File

@ -62,6 +62,7 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.LongSupplier;
@ -77,7 +78,7 @@ public class QueryShardContext extends QueryRewriteContext {
private final MapperService mapperService;
private final SimilarityService similarityService;
private final BitsetFilterCache bitsetFilterCache;
private final Function<MappedFieldType, IndexFieldData<?>> indexFieldDataService;
private final BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataService;
private final int shardId;
private final IndexReader reader;
private final String clusterAlias;
@ -101,10 +102,10 @@ public class QueryShardContext extends QueryRewriteContext {
private boolean isFilter;
public QueryShardContext(int shardId, IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache,
Function<MappedFieldType, IndexFieldData<?>> indexFieldDataLookup, MapperService mapperService,
SimilarityService similarityService, ScriptService scriptService, NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry,Client client, IndexReader reader, LongSupplier nowInMillis,
String clusterAlias) {
BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataLookup, MapperService mapperService,
SimilarityService similarityService, ScriptService scriptService, NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry, Client client, IndexReader reader, LongSupplier nowInMillis,
String clusterAlias) {
super(xContentRegistry, namedWriteableRegistry,client, nowInMillis);
this.shardId = shardId;
this.similarityService = similarityService;
@ -164,13 +165,7 @@ public class QueryShardContext extends QueryRewriteContext {
}
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
if (clusterAlias != null && IndexFieldMapper.NAME.equals(fieldType.name())) {
// this is a "hack" to make the _index field data aware of cross cluster search cluster aliases.
ConstantIndexFieldData ifd = (ConstantIndexFieldData) indexFieldDataService.apply(fieldType);
return (IFD) new ConstantIndexFieldData.Builder(m -> fullyQualifiedIndexName)
.build(indexSettings, fieldType, null, null, mapperService);
}
return (IFD) indexFieldDataService.apply(fieldType);
return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndexName);
}
public void addNamedQuery(String name, Query query) {
@ -283,7 +278,8 @@ public class QueryShardContext extends QueryRewriteContext {
public SearchLookup lookup() {
if (lookup == null) {
lookup = new SearchLookup(getMapperService(), indexFieldDataService, types);
lookup = new SearchLookup(getMapperService(),
mappedFieldType -> indexFieldDataService.apply(mappedFieldType, fullyQualifiedIndexName), types);
}
return lookup;
}

View File

@ -152,7 +152,8 @@ public class ScaledFloatFieldTypeTests extends FieldTypeTestCase {
// single-valued
ft.setName("scaled_float1");
IndexNumericFieldData fielddata = (IndexNumericFieldData) ft.fielddataBuilder().build(indexSettings, ft, null, null, null);
IndexNumericFieldData fielddata = (IndexNumericFieldData) ft.fielddataBuilder("index")
.build(indexSettings, ft, null, null, null);
assertEquals(fielddata.getNumericType(), IndexNumericFieldData.NumericType.DOUBLE);
AtomicNumericFieldData leafFieldData = fielddata.load(reader.leaves().get(0));
SortedNumericDoubleValues values = leafFieldData.getDoubleValues();
@ -162,7 +163,7 @@ public class ScaledFloatFieldTypeTests extends FieldTypeTestCase {
// multi-valued
ft.setName("scaled_float2");
fielddata = (IndexNumericFieldData) ft.fielddataBuilder().build(indexSettings, ft, null, null, null);
fielddata = (IndexNumericFieldData) ft.fielddataBuilder("index").build(indexSettings, ft, null, null, null);
leafFieldData = fielddata.load(reader.leaves().get(0));
values = leafFieldData.getDoubleValues();
assertTrue(values.advanceExact(0));

View File

@ -477,7 +477,7 @@ public class TextFieldMapperTests extends ESSingleNodeTestCase {
DocumentMapper disabledMapper = parser.parse("type", new CompressedXContent(mapping));
assertEquals(mapping, disabledMapper.mappingSource().toString());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> disabledMapper.mappers().getMapper("field").fieldType().fielddataBuilder());
() -> disabledMapper.mappers().getMapper("field").fieldType().fielddataBuilder("test"));
assertThat(e.getMessage(), containsString("Fielddata is disabled"));
mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
@ -490,7 +490,7 @@ public class TextFieldMapperTests extends ESSingleNodeTestCase {
DocumentMapper enabledMapper = parser.parse("type", new CompressedXContent(mapping));
assertEquals(mapping, enabledMapper.mappingSource().toString());
enabledMapper.mappers().getMapper("field").fieldType().fielddataBuilder(); // no exception this time
enabledMapper.mappers().getMapper("field").fieldType().fielddataBuilder("test"); // no exception this time
String illegalMapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("field")

View File

@ -76,7 +76,7 @@ public class TypeFieldMapperTests extends ESSingleNodeTestCase {
w.close();
MappedFieldType ft = mapperService.fullName(TypeFieldMapper.NAME);
IndexOrdinalsFieldData fd = (IndexOrdinalsFieldData) ft.fielddataBuilder().build(mapperService.getIndexSettings(),
IndexOrdinalsFieldData fd = (IndexOrdinalsFieldData) ft.fielddataBuilder("test").build(mapperService.getIndexSettings(),
ft, new IndexFieldDataCache.None(), new NoneCircuitBreakerService(), mapperService);
AtomicOrdinalsFieldData afd = fd.load(r.leaves().get(0));
SortedSetDocValues values = afd.getOrdinalsValues();

View File

@ -20,7 +20,6 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.lucene.search.Queries;
@ -35,7 +34,6 @@ import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.TextFieldMapper;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matcher;
import org.hamcrest.Matchers;
import java.io.IOException;
@ -64,8 +62,8 @@ public class QueryShardContextTests extends ESTestCase {
final long nowInMillis = randomNonNegativeLong();
QueryShardContext context = new QueryShardContext(
0, indexSettings, null, mappedFieldType ->
mappedFieldType.fielddataBuilder().build(indexSettings, mappedFieldType, null, null, null)
0, indexSettings, null, (mappedFieldType, idxName) ->
mappedFieldType.fielddataBuilder(idxName).build(indexSettings, mappedFieldType, null, null, null)
, mapperService, null, null, xContentRegistry(), writableRegistry(), null, null,
() -> nowInMillis, null);
@ -109,8 +107,8 @@ public class QueryShardContextTests extends ESTestCase {
IndexFieldMapper mapper = new IndexFieldMapper.Builder(null).build(ctx);
final String clusterAlias = randomBoolean() ? null : "remote_cluster";
QueryShardContext context = new QueryShardContext(
0, indexSettings, null, mappedFieldType ->
mappedFieldType.fielddataBuilder().build(indexSettings, mappedFieldType, null, null, mapperService)
0, indexSettings, null, (mappedFieldType, indexname) ->
mappedFieldType.fielddataBuilder(indexname).build(indexSettings, mappedFieldType, null, null, mapperService)
, mapperService, null, null, xContentRegistry(), writableRegistry(), null, null,
() -> nowInMillis, clusterAlias);

View File

@ -88,7 +88,7 @@ public class MetaJoinFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder();
}

View File

@ -111,7 +111,7 @@ public final class ParentIdFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder();
}

View File

@ -223,7 +223,7 @@ public final class ParentJoinFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder();
}

View File

@ -659,7 +659,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
@Override
@SuppressWarnings("unchecked")
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
IndexFieldData.Builder builder = fieldType.fielddataBuilder();
IndexFieldData.Builder builder = fieldType.fielddataBuilder(shardContext.getFullyQualifiedIndexName());
IndexFieldDataCache cache = new IndexFieldDataCache.None();
CircuitBreakerService circuitBreaker = new NoneCircuitBreakerService();
return (IFD) builder.build(shardContext.getIndexSettings(), fieldType, cache, circuitBreaker,

View File

@ -128,7 +128,7 @@ public class ICUCollationKeywordFieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder();
}

View File

@ -122,7 +122,7 @@ public class Murmur3FieldMapper extends FieldMapper {
}
@Override
public IndexFieldData.Builder fielddataBuilder() {
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
failIfNoDocValues();
return new DocValuesIndexFieldData.Builder().numericType(NumericType.LONG);
}

View File

@ -44,7 +44,6 @@ import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
import org.elasticsearch.index.cache.bitset.BitsetFilterCache.Listener;
import org.elasticsearch.index.cache.query.DisabledQueryCache;
import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.IndexFieldDataCache;
import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.mapper.ContentPath;
@ -202,9 +201,9 @@ public abstract class AggregatorTestCase extends ESTestCase {
when(queryShardContext.getMapperService()).thenReturn(mapperService);
for (MappedFieldType fieldType : fieldTypes) {
when(queryShardContext.fieldMapper(fieldType.name())).thenReturn(fieldType);
when(queryShardContext.getForField(fieldType)).then(invocation -> fieldType.fielddataBuilder().build(
mapperService.getIndexSettings(), fieldType, new IndexFieldDataCache.None(), circuitBreakerService,
mapperService));
when(queryShardContext.getForField(fieldType)).then(invocation -> fieldType.fielddataBuilder(mapperService.getIndexSettings()
.getIndex().getName())
.build(mapperService.getIndexSettings(), fieldType, new IndexFieldDataCache.None(), circuitBreakerService, mapperService));
}
NestedScope nestedScope = new NestedScope();
when(queryShardContext.isFilter()).thenCallRealMethod();