Skip to content

Commit 2a9e474

Browse files
lktslkts
authored
Rework fix for stale data in synthetic source to improve performance (#112480)
1 parent 3d389ce commit 2a9e474

File tree

17 files changed

+149
-148
lines changed

17 files changed

+149
-148
lines changed

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@
1414
import org.elasticsearch.xcontent.XContentBuilder;
1515

1616
import java.io.IOException;
17-
import java.util.Map;
18-
import java.util.stream.Stream;
1917

20-
public abstract class BinaryDocValuesSyntheticFieldLoader implements SourceLoader.SyntheticFieldLoader {
18+
public abstract class BinaryDocValuesSyntheticFieldLoader extends SourceLoader.DocValuesBasedSyntheticFieldLoader {
2119
private final String name;
2220
private BinaryDocValues values;
2321
private boolean hasValue;
@@ -28,11 +26,6 @@ protected BinaryDocValuesSyntheticFieldLoader(String name) {
2826

2927
protected abstract void writeValue(XContentBuilder b, BytesRef value) throws IOException;
3028

31-
@Override
32-
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
33-
return Stream.of();
34-
}
35-
3629
@Override
3730
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
3831
values = leafReader.getBinaryDocValues(name);

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

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,6 @@ public CompositeSyntheticFieldLoader(String leafFieldName, String fullFieldName,
5050
@Override
5151
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
5252
return parts.stream().flatMap(Layer::storedFieldLoaders).map(e -> Map.entry(e.getKey(), new StoredFieldLoader() {
53-
@Override
54-
public void advanceToDoc(int docId) {
55-
storedFieldLoadersHaveValues = false;
56-
e.getValue().advanceToDoc(docId);
57-
}
58-
5953
@Override
6054
public void load(List<Object> newValues) {
6155
storedFieldLoadersHaveValues = true;
@@ -79,8 +73,6 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
7973
}
8074

8175
return docId -> {
82-
this.docValuesLoadersHaveValues = false;
83-
8476
boolean hasDocs = false;
8577
for (var loader : loaders) {
8678
hasDocs |= loader.advanceToDoc(docId);
@@ -117,6 +109,18 @@ public void write(XContentBuilder b) throws IOException {
117109
part.write(b);
118110
}
119111
b.endArray();
112+
softReset();
113+
}
114+
115+
private void softReset() {
116+
storedFieldLoadersHaveValues = false;
117+
docValuesLoadersHaveValues = false;
118+
}
119+
120+
@Override
121+
public void reset() {
122+
softReset();
123+
parts.forEach(SourceLoader.SyntheticFieldLoader::reset);
120124
}
121125

122126
@Override
@@ -139,6 +143,19 @@ public interface Layer extends SourceLoader.SyntheticFieldLoader {
139143
long valueCount();
140144
}
141145

146+
public interface DocValuesLayer extends Layer {
147+
@Override
148+
default Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
149+
return Stream.empty();
150+
}
151+
152+
@Override
153+
default void reset() {
154+
// Not applicable to loaders using only doc values
155+
// since DocValuesLoader#advanceToDoc will reset the state anyway.
156+
}
157+
}
158+
142159
/**
143160
* Layer that loads malformed values stored in a dedicated field with a conventional name.
144161
* @see IgnoreMalformedStoredValues
@@ -177,17 +194,7 @@ public long valueCount() {
177194

178195
@Override
179196
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
180-
return Stream.of(Map.entry(fieldName, new SourceLoader.SyntheticFieldLoader.StoredFieldLoader() {
181-
@Override
182-
public void advanceToDoc(int docId) {
183-
values = emptyList();
184-
}
185-
186-
@Override
187-
public void load(List<Object> newValues) {
188-
values = newValues;
189-
}
190-
}));
197+
return Stream.of(Map.entry(fieldName, newValues -> values = newValues));
191198
}
192199

193200
@Override
@@ -205,6 +212,12 @@ public void write(XContentBuilder b) throws IOException {
205212
for (Object v : values) {
206213
writeValue(v, b);
207214
}
215+
reset();
216+
}
217+
218+
@Override
219+
public void reset() {
220+
values = emptyList();
208221
}
209222

210223
/**

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020

2121
import java.io.IOException;
2222
import java.util.Collections;
23-
import java.util.Map;
24-
import java.util.stream.Stream;
2523

2624
/** Mapper for the doc_count field. */
2725
public class DocCountFieldMapper extends MetadataFieldMapper {
@@ -139,15 +137,10 @@ public static PostingsEnum leafLookup(LeafReader reader) throws IOException {
139137
return reader.postings(TERM);
140138
}
141139

142-
private static class SyntheticFieldLoader implements SourceLoader.SyntheticFieldLoader {
140+
private static class SyntheticFieldLoader extends SourceLoader.DocValuesBasedSyntheticFieldLoader {
143141
private PostingsEnum postings;
144142
private boolean hasValue;
145143

146-
@Override
147-
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
148-
return Stream.empty();
149-
}
150-
151144
@Override
152145
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
153146
postings = leafLookup(leafReader);

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ public static IgnoreMalformedStoredValues stored(String fieldName) {
7979
*/
8080
public abstract void write(XContentBuilder b) throws IOException;
8181

82+
/**
83+
* Remove stored values for this document and return to clean state to process next document.
84+
*/
85+
public abstract void reset();
86+
8287
private static final Empty EMPTY = new Empty();
8388

8489
private static class Empty extends IgnoreMalformedStoredValues {
@@ -94,6 +99,9 @@ public int count() {
9499

95100
@Override
96101
public void write(XContentBuilder b) throws IOException {}
102+
103+
@Override
104+
public void reset() {}
97105
}
98106

99107
private static class Stored extends IgnoreMalformedStoredValues {
@@ -107,17 +115,7 @@ private static class Stored extends IgnoreMalformedStoredValues {
107115

108116
@Override
109117
public Stream<Map.Entry<String, SourceLoader.SyntheticFieldLoader.StoredFieldLoader>> storedFieldLoaders() {
110-
return Stream.of(Map.entry(name(fieldName), new SourceLoader.SyntheticFieldLoader.StoredFieldLoader() {
111-
@Override
112-
public void advanceToDoc(int docId) {
113-
values = emptyList();
114-
}
115-
116-
@Override
117-
public void load(List<Object> newValues) {
118-
values = newValues;
119-
}
120-
}));
118+
return Stream.of(Map.entry(name(fieldName), newValues -> values = newValues));
121119
}
122120

123121
@Override
@@ -134,6 +132,11 @@ public void write(XContentBuilder b) throws IOException {
134132
b.value(v);
135133
}
136134
}
135+
reset();
136+
}
137+
138+
@Override
139+
public void reset() {
137140
values = emptyList();
138141
}
139142
}

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.util.Optional;
3232
import java.util.function.Function;
3333
import java.util.function.Supplier;
34-
import java.util.stream.Stream;
3534

3635
import static org.elasticsearch.index.mapper.SourceFieldMetrics.NOOP;
3736

@@ -393,7 +392,7 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
393392
);
394393
}
395394

396-
private class NestedSyntheticFieldLoader implements SourceLoader.SyntheticFieldLoader {
395+
private class NestedSyntheticFieldLoader extends SourceLoader.DocValuesBasedSyntheticFieldLoader {
397396
private final org.elasticsearch.index.fieldvisitor.StoredFieldLoader storedFieldLoader;
398397
private final SourceLoader sourceLoader;
399398
private final Supplier<BitSetProducer> parentBitSetProducer;
@@ -415,11 +414,6 @@ private NestedSyntheticFieldLoader(
415414
this.childFilter = childFilter;
416415
}
417416

418-
@Override
419-
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
420-
return Stream.of();
421-
}
422-
423417
@Override
424418
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
425419
this.children.clear();

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

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -841,18 +841,9 @@ private SyntheticSourceFieldLoader(List<SourceLoader.SyntheticFieldLoader> field
841841
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
842842
return fields.stream()
843843
.flatMap(SourceLoader.SyntheticFieldLoader::storedFieldLoaders)
844-
.map(e -> Map.entry(e.getKey(), new StoredFieldLoader() {
845-
@Override
846-
public void advanceToDoc(int docId) {
847-
storedFieldLoadersHaveValues = false;
848-
e.getValue().advanceToDoc(docId);
849-
}
850-
851-
@Override
852-
public void load(List<Object> newValues) {
853-
storedFieldLoadersHaveValues = true;
854-
e.getValue().load(newValues);
855-
}
844+
.map(e -> Map.entry(e.getKey(), newValues -> {
845+
storedFieldLoadersHaveValues = true;
846+
e.getValue().load(newValues);
856847
}));
857848
}
858849

@@ -880,8 +871,6 @@ private ObjectDocValuesLoader(List<DocValuesLoader> loaders) {
880871

881872
@Override
882873
public boolean advanceToDoc(int docId) throws IOException {
883-
docValuesLoadersHaveValues = false;
884-
885874
boolean anyLeafHasDocValues = false;
886875
for (DocValuesLoader docValueLoader : loaders) {
887876
boolean leafHasValue = docValueLoader.advanceToDoc(docId);
@@ -930,8 +919,14 @@ public void write(XContentBuilder b) throws IOException {
930919
}
931920
for (SourceLoader.SyntheticFieldLoader field : fields) {
932921
if (field.hasValue()) {
933-
// Skip if the field source is stored separately, to avoid double-printing.
934-
orderedFields.computeIfAbsent(field.fieldName(), k -> new FieldWriter.FieldLoader(field));
922+
if (orderedFields.containsKey(field.fieldName()) == false) {
923+
orderedFields.put(field.fieldName(), new FieldWriter.FieldLoader(field));
924+
} else {
925+
// Skip if the field source is stored separately, to avoid double-printing.
926+
// Make sure to reset the state of loader so that values stored inside will not
927+
// be used after this document is finished.
928+
field.reset();
929+
}
935930
}
936931
}
937932

@@ -947,12 +942,30 @@ public void write(XContentBuilder b) throws IOException {
947942
}
948943
}
949944
b.endObject();
945+
softReset();
950946
}
951947

952-
@Override
953-
public boolean setIgnoredValues(Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields) {
948+
/**
949+
* reset() is expensive since it will descend the hierarchy and reset the loader
950+
* of every field.
951+
* We perform a reset of a child field inside write() only when it is needed.
952+
* We know that either write() or reset() was called for every field,
953+
* so in the end of write() we can do this soft reset only.
954+
*/
955+
private void softReset() {
956+
storedFieldLoadersHaveValues = false;
957+
docValuesLoadersHaveValues = false;
954958
ignoredValuesPresent = false;
959+
}
960+
961+
@Override
962+
public void reset() {
963+
softReset();
964+
fields.forEach(SourceLoader.SyntheticFieldLoader::reset);
965+
}
955966

967+
@Override
968+
public boolean setIgnoredValues(Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields) {
956969
if (objectsWithIgnoredFields == null || objectsWithIgnoredFields.isEmpty()) {
957970
return false;
958971
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ public void write(XContentBuilder b) throws IOException {
107107
}
108108
}
109109

110+
@Override
111+
public void reset() {
112+
ignoreMalformedValues.reset();
113+
}
114+
110115
private interface Values {
111116
int count();
112117

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,11 @@
1919

2020
import java.io.IOException;
2121
import java.util.Arrays;
22-
import java.util.Map;
23-
import java.util.stream.Stream;
2422

2523
/**
2624
* Load {@code _source} fields from {@link SortedSetDocValues}.
2725
*/
28-
public abstract class SortedSetDocValuesSyntheticFieldLoaderLayer implements CompositeSyntheticFieldLoader.Layer {
26+
public abstract class SortedSetDocValuesSyntheticFieldLoaderLayer implements CompositeSyntheticFieldLoader.DocValuesLayer {
2927
private static final Logger logger = LogManager.getLogger(SortedSetDocValuesSyntheticFieldLoaderLayer.class);
3028

3129
private final String name;
@@ -44,11 +42,6 @@ public String fieldName() {
4442
return name;
4543
}
4644

47-
@Override
48-
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
49-
return Stream.of();
50-
}
51-
5245
@Override
5346
public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) throws IOException {
5447
SortedSetDocValues dv = DocValues.getSortedSet(reader, name);

0 commit comments

Comments
 (0)