Skip to content

Commit 0fd5587

Browse files
Kubik42Kubik42
committed
Fixed geo point block loader slowness
1 parent 40f90fb commit 0fd5587

File tree

3 files changed

+201
-15
lines changed

3 files changed

+201
-15
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
206206
};
207207
}
208208

209-
private class GeometriesFallbackSyntheticSourceReader implements FallbackSyntheticSourceBlockLoader.Reader<BytesRef> {
209+
class GeometriesFallbackSyntheticSourceReader implements FallbackSyntheticSourceBlockLoader.Reader<BytesRef> {
210210
private final Function<List<T>, List<Object>> formatter;
211211

212212
private GeometriesFallbackSyntheticSourceReader() {

‎server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@
7070
import java.util.function.Function;
7171

7272
import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.DOC_VALUES;
73+
import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.NONE;
74+
import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.STORED;
7375

7476
/**
7577
* Field Mapper for geo_point types.
@@ -387,7 +389,7 @@ public static class GeoPointFieldType extends AbstractPointFieldType<GeoPoint> i
387389
private final IndexMode indexMode;
388390
private final boolean isSyntheticSource;
389391

390-
private GeoPointFieldType(
392+
GeoPointFieldType(
391393
String name,
392394
boolean indexed,
393395
boolean stored,
@@ -545,24 +547,33 @@ public TimeSeriesParams.MetricType getMetricType() {
545547

546548
@Override
547549
public BlockLoader blockLoader(BlockLoaderContext blContext) {
548-
if (blContext.fieldExtractPreference() == DOC_VALUES && hasDocValues()) {
550+
boolean noPreference = blContext.fieldExtractPreference() == NONE;
551+
552+
// load from doc_values
553+
boolean preferToLoadFromDocValues = blContext.fieldExtractPreference() == DOC_VALUES;
554+
if ((preferToLoadFromDocValues || noPreference) && hasDocValues()) {
549555
return new BlockDocValuesReader.LongsBlockLoader(name());
550556
}
551557

552-
// There are two scenarios possible once we arrive here:
553-
//
554-
// * Stored source - we'll just use blockLoaderFromSource
555-
// * Synthetic source. However, because of the fieldExtractPreference() check above it is still possible that doc_values are
556-
// present here.
557-
// So we have two subcases:
558-
// - doc_values are enabled - _ignored_source field does not exist since we have doc_values. We will use
559-
// blockLoaderFromSource which reads "native" synthetic source.
560-
// - doc_values are disabled - we know that _ignored_source field is present and use a special block loader unless it's a multi
561-
// field.
562-
if (isSyntheticSource && hasDocValues() == false && blContext.parentField(name()) == null) {
563-
return blockLoaderFromFallbackSyntheticSource(blContext);
558+
// load from a stored field
559+
boolean preferToLoadFromStoredFields = blContext.fieldExtractPreference() == STORED;
560+
if ((preferToLoadFromStoredFields || noPreference) && isStored()) {
561+
return new BlockStoredFieldsReader.BytesFromStringsBlockLoader(name());
562+
}
563+
564+
if (isSyntheticSource) {
565+
// if doc_values are unavailable, then the field must've been written to _ignored_source, so load it from there
566+
// assuming the field is not a multi field, since those don't have fallback synthetic source
567+
if (hasDocValues() == false && blContext.parentField(name()) == null) {
568+
return blockLoaderFromFallbackSyntheticSource(blContext);
569+
} else if (hasDocValues()) {
570+
// otherwise, the loading preference must've failed the doc_values check above and doc_values exist
571+
// technically, we're ignoring the preference here, but its a worthy trade-off for speed
572+
return new BlockDocValuesReader.LongsBlockLoader(name());
573+
}
564574
}
565575

576+
// otherwise, load from _source (synthetic or otherwise) - very slow
566577
return blockLoaderFromSource(blContext);
567578
}
568579
}

‎server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,28 @@
1010
package org.elasticsearch.index.mapper;
1111

1212
import org.apache.lucene.tests.geo.GeoTestUtil;
13+
import org.elasticsearch.cluster.metadata.IndexMetadata;
1314
import org.elasticsearch.common.geo.GeoPoint;
1415
import org.elasticsearch.common.geo.SimpleFeatureFactory;
16+
import org.elasticsearch.common.settings.Settings;
1517
import org.elasticsearch.geometry.Point;
1618
import org.elasticsearch.geometry.utils.WellKnownBinary;
19+
import org.elasticsearch.index.IndexMode;
20+
import org.elasticsearch.index.IndexSettings;
1721
import org.elasticsearch.index.IndexVersion;
1822
import org.elasticsearch.script.ScriptCompiler;
1923

2024
import java.io.IOException;
2125
import java.nio.ByteOrder;
2226
import java.util.ArrayList;
27+
import java.util.Collections;
2328
import java.util.List;
2429
import java.util.Map;
2530

2631
import static org.hamcrest.Matchers.equalTo;
32+
import static org.hamcrest.Matchers.instanceOf;
33+
import static org.mockito.Mockito.doReturn;
34+
import static org.mockito.Mockito.mock;
2735

2836
public class GeoPointFieldTypeTests extends FieldTypeTestCase {
2937

@@ -147,4 +155,171 @@ public void testFetchVectorTile() throws IOException {
147155
assertThat(sourceValue.size(), equalTo(1));
148156
assertThat(sourceValue.get(0), equalTo(featureFactory.points(geoPoints)));
149157
}
158+
159+
public void testBlockLoaderWhenDocValuesAreEnabledAndThePreferenceIsToUseDocValues() {
160+
// given
161+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType("potato");
162+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
163+
doReturn(MappedFieldType.FieldExtractPreference.DOC_VALUES).when(blContextMock).fieldExtractPreference();
164+
165+
// when
166+
BlockLoader loader = fieldType.blockLoader(blContextMock);
167+
168+
// then
169+
// verify that we use the correct block value reader
170+
assertThat(loader, instanceOf(BlockDocValuesReader.LongsBlockLoader.class));
171+
}
172+
173+
public void testBlockLoaderWhenDocValuesAreEnabledAndThereIsNoPreference() {
174+
// given
175+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType("potato");
176+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
177+
doReturn(MappedFieldType.FieldExtractPreference.NONE).when(blContextMock).fieldExtractPreference();
178+
179+
// when
180+
BlockLoader loader = fieldType.blockLoader(blContextMock);
181+
182+
// then
183+
// verify that we use the correct block value reader
184+
assertThat(loader, instanceOf(BlockDocValuesReader.LongsBlockLoader.class));
185+
}
186+
187+
public void testBlockLoaderWhenFieldIsStoredAndThePreferenceIsToUseStoredFields() {
188+
// given
189+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType(
190+
"potato",
191+
true,
192+
true,
193+
false,
194+
null,
195+
null,
196+
null,
197+
Collections.emptyMap(),
198+
TimeSeriesParams.MetricType.COUNTER,
199+
IndexMode.LOGSDB,
200+
false
201+
);
202+
203+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
204+
doReturn(MappedFieldType.FieldExtractPreference.STORED).when(blContextMock).fieldExtractPreference();
205+
206+
// when
207+
BlockLoader loader = fieldType.blockLoader(blContextMock);
208+
209+
// then
210+
// verify that we use the correct block value reader
211+
assertThat(loader, instanceOf(BlockStoredFieldsReader.BytesFromStringsBlockLoader.class));
212+
}
213+
214+
public void testBlockLoaderWhenFieldIsStoredAndThereIsNoPreference() {
215+
// given
216+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType(
217+
"potato",
218+
true,
219+
true,
220+
false,
221+
null,
222+
null,
223+
null,
224+
Collections.emptyMap(),
225+
TimeSeriesParams.MetricType.COUNTER,
226+
IndexMode.LOGSDB,
227+
false
228+
);
229+
230+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
231+
doReturn(MappedFieldType.FieldExtractPreference.NONE).when(blContextMock).fieldExtractPreference();
232+
233+
// when
234+
BlockLoader loader = fieldType.blockLoader(blContextMock);
235+
236+
// then
237+
// verify that we use the correct block value reader
238+
assertThat(loader, instanceOf(BlockStoredFieldsReader.BytesFromStringsBlockLoader.class));
239+
}
240+
241+
public void testBlockLoaderWhenSyntheticSourceIsEnabledAndFieldIsStoredInIgnoredSource() {
242+
// given
243+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType(
244+
"potato",
245+
true,
246+
false,
247+
false,
248+
null,
249+
null,
250+
null,
251+
Collections.emptyMap(),
252+
TimeSeriesParams.MetricType.COUNTER,
253+
IndexMode.LOGSDB,
254+
true
255+
);
256+
257+
Settings settings = Settings.builder()
258+
.put("index.mapping.source.mode", "synthetic")
259+
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
260+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
261+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
262+
.build();
263+
IndexSettings indexSettings = new IndexSettings(IndexMetadata.builder("index").settings(settings).build(), settings);
264+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
265+
doReturn(MappedFieldType.FieldExtractPreference.NONE).when(blContextMock).fieldExtractPreference();
266+
doReturn(indexSettings).when(blContextMock).indexSettings();
267+
268+
// when
269+
BlockLoader loader = fieldType.blockLoader(blContextMock);
270+
271+
// then
272+
// verify that we use the correct block value reader
273+
assertThat(loader, instanceOf(FallbackSyntheticSourceBlockLoader.class));
274+
}
275+
276+
public void testBlockLoaderWhenSyntheticSourceAndDocValuesAreEnabled() {
277+
// given
278+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType(
279+
"potato",
280+
true,
281+
false,
282+
true,
283+
null,
284+
null,
285+
null,
286+
Collections.emptyMap(),
287+
TimeSeriesParams.MetricType.COUNTER,
288+
IndexMode.LOGSDB,
289+
true
290+
);
291+
292+
Settings settings = Settings.builder()
293+
.put("index.mapping.source.mode", "synthetic")
294+
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
295+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
296+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
297+
.build();
298+
IndexSettings indexSettings = new IndexSettings(IndexMetadata.builder("index").settings(settings).build(), settings);
299+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
300+
doReturn(MappedFieldType.FieldExtractPreference.NONE).when(blContextMock).fieldExtractPreference();
301+
doReturn(indexSettings).when(blContextMock).indexSettings();
302+
303+
// when
304+
BlockLoader loader = fieldType.blockLoader(blContextMock);
305+
306+
// then
307+
// verify that we use the correct block value reader
308+
assertThat(loader, instanceOf(BlockDocValuesReader.LongsBlockLoader.class));
309+
}
310+
311+
public void testBlockLoaderFallsBackToSource() {
312+
// given
313+
GeoPointFieldMapper.GeoPointFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType("potato");
314+
MappedFieldType.BlockLoaderContext blContextMock = mock(MappedFieldType.BlockLoaderContext.class);
315+
doReturn(MappedFieldType.FieldExtractPreference.EXTRACT_SPATIAL_BOUNDS).when(blContextMock).fieldExtractPreference();
316+
317+
// when
318+
BlockLoader loader = fieldType.blockLoader(blContextMock);
319+
320+
// then
321+
// verify that we use the correct block value reader
322+
assertThat(loader, instanceOf(BlockSourceReader.GeometriesBlockLoader.class));
323+
}
324+
150325
}

0 commit comments

Comments
 (0)