Skip to content

Commit 888ec64

Browse files
lktslkts
authored
Rework fix for stale data in synthetic source to improve performance (elastic#112480) (elastic#112524)
(cherry picked from commit 2a9e474)
1 parent 6a76720 commit 888ec64

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
@@ -30,7 +30,6 @@
3030
import java.util.Map;
3131
import java.util.function.Function;
3232
import java.util.function.Supplier;
33-
import java.util.stream.Stream;
3433

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

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

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

417-
@Override
418-
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
419-
return Stream.of();
420-
}
421-
422416
@Override
423417
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
424418
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
@@ -782,18 +782,9 @@ private SyntheticSourceFieldLoader(List<SourceLoader.SyntheticFieldLoader> field
782782
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
783783
return fields.stream()
784784
.flatMap(SourceLoader.SyntheticFieldLoader::storedFieldLoaders)
785-
.map(e -> Map.entry(e.getKey(), new StoredFieldLoader() {
786-
@Override
787-
public void advanceToDoc(int docId) {
788-
storedFieldLoadersHaveValues = false;
789-
e.getValue().advanceToDoc(docId);
790-
}
791-
792-
@Override
793-
public void load(List<Object> newValues) {
794-
storedFieldLoadersHaveValues = true;
795-
e.getValue().load(newValues);
796-
}
785+
.map(e -> Map.entry(e.getKey(), newValues -> {
786+
storedFieldLoadersHaveValues = true;
787+
e.getValue().load(newValues);
797788
}));
798789
}
799790

@@ -821,8 +812,6 @@ private ObjectDocValuesLoader(List<DocValuesLoader> loaders) {
821812

822813
@Override
823814
public boolean advanceToDoc(int docId) throws IOException {
824-
docValuesLoadersHaveValues = false;
825-
826815
boolean anyLeafHasDocValues = false;
827816
for (DocValuesLoader docValueLoader : loaders) {
828817
boolean leafHasValue = docValueLoader.advanceToDoc(docId);
@@ -871,8 +860,14 @@ public void write(XContentBuilder b) throws IOException {
871860
}
872861
for (SourceLoader.SyntheticFieldLoader field : fields) {
873862
if (field.hasValue()) {
874-
// Skip if the field source is stored separately, to avoid double-printing.
875-
orderedFields.computeIfAbsent(field.fieldName(), k -> new FieldWriter.FieldLoader(field));
863+
if (orderedFields.containsKey(field.fieldName()) == false) {
864+
orderedFields.put(field.fieldName(), new FieldWriter.FieldLoader(field));
865+
} else {
866+
// Skip if the field source is stored separately, to avoid double-printing.
867+
// Make sure to reset the state of loader so that values stored inside will not
868+
// be used after this document is finished.
869+
field.reset();
870+
}
876871
}
877872
}
878873

@@ -888,12 +883,30 @@ public void write(XContentBuilder b) throws IOException {
888883
}
889884
}
890885
b.endObject();
886+
softReset();
891887
}
892888

893-
@Override
894-
public boolean setIgnoredValues(Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields) {
889+
/**
890+
* reset() is expensive since it will descend the hierarchy and reset the loader
891+
* of every field.
892+
* We perform a reset of a child field inside write() only when it is needed.
893+
* We know that either write() or reset() was called for every field,
894+
* so in the end of write() we can do this soft reset only.
895+
*/
896+
private void softReset() {
897+
storedFieldLoadersHaveValues = false;
898+
docValuesLoadersHaveValues = false;
895899
ignoredValuesPresent = false;
900+
}
901+
902+
@Override
903+
public void reset() {
904+
softReset();
905+
fields.forEach(SourceLoader.SyntheticFieldLoader::reset);
906+
}
896907

908+
@Override
909+
public boolean setIgnoredValues(Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields) {
897910
if (objectsWithIgnoredFields == null || objectsWithIgnoredFields.isEmpty()) {
898911
return false;
899912
}

‎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)