Skip to content

Commit 152af43

Browse files
iveraseiverase
authored
[8.x] Stop caching source map on SearchHit#getSourceMap (elastic#119888) (elastic#120743)
This call has the side effect that if you are iterating a number of hits calling this method, you will be increasing the memory usage by a non trivial number which in most of cases is unwanted. Therefore this commit removes this caching all together and add an assertion so the method is call once during the lifetime of the object. backport elastic#119888
1 parent 109b6ff commit 152af43

File tree

55 files changed

+480
-375
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+480
-375
lines changed

‎modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/ChildQuerySearchIT.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,9 @@ public void testSimpleChildQuery() throws Exception {
184184
assertNoFailuresAndResponse(prepareSearch("test").setQuery(idsQuery().addIds("c1")), response -> {
185185
assertThat(response.getHits().getTotalHits().value, equalTo(1L));
186186
assertThat(response.getHits().getAt(0).getId(), equalTo("c1"));
187-
assertThat(extractValue("join_field.name", response.getHits().getAt(0).getSourceAsMap()), equalTo("child"));
188-
assertThat(extractValue("join_field.parent", response.getHits().getAt(0).getSourceAsMap()), equalTo("p1"));
187+
Map<String, Object> source = response.getHits().getAt(0).getSourceAsMap();
188+
assertThat(extractValue("join_field.name", source), equalTo("child"));
189+
assertThat(extractValue("join_field.parent", source), equalTo("p1"));
189190

190191
});
191192

@@ -197,11 +198,13 @@ public void testSimpleChildQuery() throws Exception {
197198
response -> {
198199
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
199200
assertThat(response.getHits().getAt(0).getId(), anyOf(equalTo("c1"), equalTo("c2")));
200-
assertThat(extractValue("join_field.name", response.getHits().getAt(0).getSourceAsMap()), equalTo("child"));
201-
assertThat(extractValue("join_field.parent", response.getHits().getAt(0).getSourceAsMap()), equalTo("p1"));
201+
Map<String, Object> source0 = response.getHits().getAt(0).getSourceAsMap();
202+
assertThat(extractValue("join_field.name", source0), equalTo("child"));
203+
assertThat(extractValue("join_field.parent", source0), equalTo("p1"));
202204
assertThat(response.getHits().getAt(1).getId(), anyOf(equalTo("c1"), equalTo("c2")));
203-
assertThat(extractValue("join_field.name", response.getHits().getAt(1).getSourceAsMap()), equalTo("child"));
204-
assertThat(extractValue("join_field.parent", response.getHits().getAt(1).getSourceAsMap()), equalTo("p1"));
205+
Map<String, Object> source1 = response.getHits().getAt(1).getSourceAsMap();
206+
assertThat(extractValue("join_field.name", source1), equalTo("child"));
207+
assertThat(extractValue("join_field.parent", source1), equalTo("p1"));
205208
}
206209
);
207210

‎server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -655,8 +655,9 @@ public void testFetchFeatures() throws IOException {
655655
assertThat(hit.field("field2").getValue(), equalTo(2.71f));
656656
assertThat(hit.field("script").getValue().toString(), equalTo("5"));
657657

658-
assertThat(hit.getSourceAsMap().size(), equalTo(1));
659-
assertThat(hit.getSourceAsMap().get("text").toString(), equalTo("some text to entertain"));
658+
Map<String, Object> source = hit.getSourceAsMap();
659+
assertThat(source.size(), equalTo(1));
660+
assertThat(source.get("text").toString(), equalTo("some text to entertain"));
660661
assertEquals("some text to entertain", hit.getFields().get("text").getValue());
661662
assertEquals("some text to entertain", hit.getFields().get("text_stored_lookup").getValue());
662663
}
@@ -927,8 +928,9 @@ public void testNestedFetchFeatures() {
927928
field = searchHit.field("script");
928929
assertThat(field.getValue().toString(), equalTo("5"));
929930

930-
assertThat(searchHit.getSourceAsMap().size(), equalTo(1));
931-
assertThat(extractValue("message", searchHit.getSourceAsMap()), equalTo("some comment"));
931+
Map<String, Object> source = searchHit.getSourceAsMap();
932+
assertThat(source.size(), equalTo(1));
933+
assertThat(extractValue("message", source), equalTo("some comment"));
932934
}
933935
);
934936
}

‎server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -490,8 +490,9 @@ public void testNestedMultipleLayers() throws Exception {
490490
response -> {
491491
SearchHits innerHits = response.getHits().getAt(0).getInnerHits().get("comments");
492492
innerHits = innerHits.getAt(0).getInnerHits().get("remark");
493-
assertNotNull(innerHits.getAt(0).getSourceAsMap());
494-
assertFalse(innerHits.getAt(0).getSourceAsMap().isEmpty());
493+
Map<String, Object> source = innerHits.getAt(0).getSourceAsMap();
494+
assertNotNull(source);
495+
assertFalse(source.isEmpty());
495496
}
496497
);
497498
assertNoFailuresAndResponse(
@@ -507,8 +508,9 @@ public void testNestedMultipleLayers() throws Exception {
507508
response -> {
508509
SearchHits innerHits = response.getHits().getAt(0).getInnerHits().get("comments");
509510
innerHits = innerHits.getAt(0).getInnerHits().get("remark");
510-
assertNotNull(innerHits.getAt(0).getSourceAsMap());
511-
assertFalse(innerHits.getAt(0).getSourceAsMap().isEmpty());
511+
Map<String, Object> source = innerHits.getAt(0).getSourceAsMap();
512+
assertNotNull(source);
513+
assertFalse(source.isEmpty());
512514
}
513515
);
514516
}
@@ -845,16 +847,12 @@ public void testNestedSource() throws Exception {
845847
assertHitCount(response, 1);
846848

847849
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getTotalHits().value, equalTo(2L));
848-
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().size(), equalTo(1));
849-
assertThat(
850-
response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().get("message"),
851-
equalTo("fox eat quick")
852-
);
853-
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(1).getSourceAsMap().size(), equalTo(1));
854-
assertThat(
855-
response.getHits().getAt(0).getInnerHits().get("comments").getAt(1).getSourceAsMap().get("message"),
856-
equalTo("fox ate rabbit x y z")
857-
);
850+
Map<String, Object> source0 = response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap();
851+
assertThat(source0.size(), equalTo(1));
852+
assertThat(source0.get("message"), equalTo("fox eat quick"));
853+
Map<String, Object> source1 = response.getHits().getAt(0).getInnerHits().get("comments").getAt(1).getSourceAsMap();
854+
assertThat(source1.size(), equalTo(1));
855+
assertThat(source1.get("message"), equalTo("fox ate rabbit x y z"));
858856
}
859857
);
860858

@@ -866,16 +864,12 @@ public void testNestedSource() throws Exception {
866864
assertHitCount(response, 1);
867865

868866
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getTotalHits().value, equalTo(2L));
869-
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().size(), equalTo(2));
870-
assertThat(
871-
response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().get("message"),
872-
equalTo("fox eat quick")
873-
);
874-
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().size(), equalTo(2));
875-
assertThat(
876-
response.getHits().getAt(0).getInnerHits().get("comments").getAt(1).getSourceAsMap().get("message"),
877-
equalTo("fox ate rabbit x y z")
878-
);
867+
Map<String, Object> source0 = response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap();
868+
assertThat(source0.size(), equalTo(2));
869+
assertThat(source0.get("message"), equalTo("fox eat quick"));
870+
Map<String, Object> source1 = response.getHits().getAt(0).getInnerHits().get("comments").getAt(1).getSourceAsMap();
871+
assertThat(source1.size(), equalTo(2));
872+
assertThat(source1.get("message"), equalTo("fox ate rabbit x y z"));
879873
}
880874
);
881875

‎server/src/internalClusterTest/java/org/elasticsearch/search/searchafter/SearchAfterIT.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import java.util.Collections;
4848
import java.util.Comparator;
4949
import java.util.List;
50+
import java.util.Map;
5051

5152
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
5253
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
@@ -152,8 +153,9 @@ public void testWithNullStrings() throws InterruptedException {
152153
searchResponse -> {
153154
assertThat(searchResponse.getHits().getTotalHits().value, Matchers.equalTo(2L));
154155
assertThat(searchResponse.getHits().getHits().length, Matchers.equalTo(1));
155-
assertThat(searchResponse.getHits().getHits()[0].getSourceAsMap().get("field1"), Matchers.equalTo(100));
156-
assertThat(searchResponse.getHits().getHits()[0].getSourceAsMap().get("field2"), Matchers.equalTo("toto"));
156+
Map<String, Object> source = searchResponse.getHits().getHits()[0].getSourceAsMap();
157+
assertThat(source.get("field1"), Matchers.equalTo(100));
158+
assertThat(source.get("field2"), Matchers.equalTo("toto"));
157159
}
158160
);
159161
}
@@ -438,8 +440,9 @@ public void testScrollAndSearchAfterWithBigIndex() {
438440
int foundHits = 0;
439441
do {
440442
for (SearchHit hit : resp.getHits().getHits()) {
441-
assertNotNull(hit.getSourceAsMap());
442-
final Object timestamp = hit.getSourceAsMap().get("timestamp");
443+
Map<String, Object> source = hit.getSourceAsMap();
444+
assertNotNull(source);
445+
final Object timestamp = source.get("timestamp");
443446
assertNotNull(timestamp);
444447
assertThat(((Number) timestamp).longValue(), equalTo(timestamps.get(foundHits)));
445448
foundHits++;
@@ -469,8 +472,9 @@ public void testScrollAndSearchAfterWithBigIndex() {
469472
do {
470473
Object[] after = null;
471474
for (SearchHit hit : resp.getHits().getHits()) {
472-
assertNotNull(hit.getSourceAsMap());
473-
final Object timestamp = hit.getSourceAsMap().get("timestamp");
475+
Map<String, Object> source = hit.getSourceAsMap();
476+
assertNotNull(source);
477+
final Object timestamp = source.get("timestamp");
474478
assertNotNull(timestamp);
475479
assertThat(((Number) timestamp).longValue(), equalTo(timestamps.get(foundHits)));
476480
after = hit.getSortValues();
@@ -505,8 +509,9 @@ public void testScrollAndSearchAfterWithBigIndex() {
505509
do {
506510
Object[] after = null;
507511
for (SearchHit hit : resp.getHits().getHits()) {
508-
assertNotNull(hit.getSourceAsMap());
509-
final Object timestamp = hit.getSourceAsMap().get("timestamp");
512+
Map<String, Object> source = hit.getSourceAsMap();
513+
assertNotNull(source);
514+
final Object timestamp = source.get("timestamp");
510515
assertNotNull(timestamp);
511516
foundSeqNos.add(((Number) timestamp).longValue());
512517
after = hit.getSortValues();

‎server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,11 @@ public void testIssue8226() {
128128
.setSize(10),
129129
response -> {
130130
logClusterState();
131+
Number previous = (Number) response.getHits().getHits()[0].getSourceAsMap().get("entry");
131132
for (int j = 1; j < response.getHits().getHits().length; j++) {
132133
Number current = (Number) response.getHits().getHits()[j].getSourceAsMap().get("entry");
133-
Number previous = (Number) response.getHits().getHits()[j - 1].getSourceAsMap().get("entry");
134134
assertThat(response.toString(), current.intValue(), lessThan(previous.intValue()));
135+
previous = current;
135136
}
136137
}
137138
);
@@ -142,10 +143,11 @@ public void testIssue8226() {
142143
.setSize(10),
143144
response -> {
144145
logClusterState();
146+
Number previous = (Number) response.getHits().getHits()[0].getSourceAsMap().get("entry");
145147
for (int j = 1; j < response.getHits().getHits().length; j++) {
146148
Number current = (Number) response.getHits().getHits()[j].getSourceAsMap().get("entry");
147-
Number previous = (Number) response.getHits().getHits()[j - 1].getSourceAsMap().get("entry");
148149
assertThat(response.toString(), current.intValue(), greaterThan(previous.intValue()));
150+
previous = current;
149151
}
150152
}
151153
);

‎server/src/internalClusterTest/java/org/elasticsearch/search/source/SourceFetchingIT.java‎

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,33 @@
1111

1212
import org.elasticsearch.test.ESIntegTestCase;
1313

14+
import java.util.Map;
15+
1416
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
17+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponses;
1518
import static org.hamcrest.Matchers.notNullValue;
1619
import static org.hamcrest.Matchers.nullValue;
1720
import static org.hamcrest.core.IsEqual.equalTo;
1821

1922
public class SourceFetchingIT extends ESIntegTestCase {
23+
2024
public void testSourceDefaultBehavior() {
2125
createIndex("test");
2226
ensureGreen();
2327

2428
indexDoc("test", "1", "field", "value");
2529
refresh();
2630

27-
assertResponse(prepareSearch("test"), response -> assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue()));
31+
assertResponses(
32+
response -> assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue()),
33+
prepareSearch("test"),
34+
prepareSearch("test").addStoredField("_source")
35+
);
2836

2937
assertResponse(
3038
prepareSearch("test").addStoredField("bla"),
3139
response -> assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue())
3240
);
33-
34-
assertResponse(
35-
prepareSearch("test").addStoredField("_source"),
36-
response -> assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue())
37-
);
38-
3941
}
4042

4143
public void testSourceFiltering() {
@@ -55,20 +57,21 @@ public void testSourceFiltering() {
5557
response -> assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue())
5658
);
5759

58-
assertResponse(prepareSearch("test").setFetchSource("field1", null), response -> {
60+
assertResponses(response -> {
5961
assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue());
60-
assertThat(response.getHits().getAt(0).getSourceAsMap().size(), equalTo(1));
61-
assertThat((String) response.getHits().getAt(0).getSourceAsMap().get("field1"), equalTo("value"));
62-
});
62+
Map<String, Object> source = response.getHits().getAt(0).getSourceAsMap();
63+
assertThat(source.size(), equalTo(1));
64+
assertThat(source.get("field1"), equalTo("value"));
65+
},
66+
prepareSearch("test").setFetchSource("field1", null),
67+
prepareSearch("test").setFetchSource(new String[] { "*" }, new String[] { "field2" })
68+
);
69+
6370
assertResponse(prepareSearch("test").setFetchSource("hello", null), response -> {
6471
assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue());
6572
assertThat(response.getHits().getAt(0).getSourceAsMap().size(), equalTo(0));
6673
});
67-
assertResponse(prepareSearch("test").setFetchSource(new String[] { "*" }, new String[] { "field2" }), response -> {
68-
assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue());
69-
assertThat(response.getHits().getAt(0).getSourceAsMap().size(), equalTo(1));
70-
assertThat((String) response.getHits().getAt(0).getSourceAsMap().get("field1"), equalTo("value"));
71-
});
74+
7275
}
7376

7477
/**
@@ -82,15 +85,14 @@ public void testSourceWithWildcardFiltering() {
8285
prepareIndex("test").setId("1").setSource("field", "value").get();
8386
refresh();
8487

85-
assertResponse(prepareSearch("test").setFetchSource(new String[] { "*.notexisting", "field" }, null), response -> {
88+
assertResponses(response -> {
8689
assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue());
87-
assertThat(response.getHits().getAt(0).getSourceAsMap().size(), equalTo(1));
88-
assertThat((String) response.getHits().getAt(0).getSourceAsMap().get("field"), equalTo("value"));
89-
});
90-
assertResponse(prepareSearch("test").setFetchSource(new String[] { "field.notexisting.*", "field" }, null), response -> {
91-
assertThat(response.getHits().getAt(0).getSourceAsString(), notNullValue());
92-
assertThat(response.getHits().getAt(0).getSourceAsMap().size(), equalTo(1));
93-
assertThat((String) response.getHits().getAt(0).getSourceAsMap().get("field"), equalTo("value"));
94-
});
90+
Map<String, Object> source = response.getHits().getAt(0).getSourceAsMap();
91+
assertThat(source.size(), equalTo(1));
92+
assertThat((String) source.get("field"), equalTo("value"));
93+
},
94+
prepareSearch("test").setFetchSource(new String[] { "*.notexisting", "field" }, null),
95+
prepareSearch("test").setFetchSource(new String[] { "field.notexisting.*", "field" }, null)
96+
);
9597
}
9698
}

‎server/src/internalClusterTest/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,9 @@ public void testSuggestDocumentSourceFiltering() throws Exception {
356356
assertThat(option.getText().toString(), equalTo("suggestion" + id));
357357
assertThat(option.getHit(), hasId("" + id));
358358
assertThat(option.getHit(), hasScore((id)));
359-
assertNotNull(option.getHit().getSourceAsMap());
360-
Set<String> sourceFields = option.getHit().getSourceAsMap().keySet();
359+
Map<String, Object> source = option.getHit().getSourceAsMap();
360+
assertNotNull(source);
361+
Set<String> sourceFields = source.keySet();
361362
assertThat(sourceFields, contains("a"));
362363
assertThat(sourceFields, not(contains("b")));
363364
id--;

0 commit comments

Comments
 (0)