Skip to content

Commit

Permalink
Fix stored access in script field type tests
Browse files Browse the repository at this point in the history 
We have been getting test failures from:

DateScriptFieldTypeTests#testDistanceFeatureQuery
BooleanScriptFieldTypeTests#testUsedInScript

They are new because Lucene now offloads execution entirely to the
executor when provided, which may surface test issues that were not
issues before.

In these examples, stored fields cached in SearchLookup were reused
across threads which made us hit the lucene assertion that ensures that
stored fields are always pulled and consumed from the same thread.

The fixes are relatively simple: recreate the query, or recreate the
mock search execution context so you get a clean search lookup with no
cached stored fields. Both failures were caused by subsequent searches
reusing the same query or the same search execution context.
  • Loading branch information
@javanna
javanna committed Sep 19, 2023
1 parent 83f9941 commit 83af00b
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 45 deletions.
69 changes: 40 additions & 29 deletions server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,41 +120,52 @@ public void testUsedInScript() throws IOException {
iw.addDocument(List.of(new StoredField("_source", new BytesRef("{\"foo\": [false]}"))));
try (DirectoryReader reader = iw.getReader()) {
IndexSearcher searcher = newSearcher(reader);
SearchExecutionContext searchContext = mockContext(true, simpleMappedFieldType());
assertThat(searcher.count(new ScriptScoreQuery(new MatchAllDocsQuery(), new Script("test"), new ScoreScript.LeafFactory() {
@Override
public boolean needs_score() {
return false;
}
{
SearchExecutionContext searchContext = mockContext(true, simpleMappedFieldType());
assertThat(
searcher.count(new ScriptScoreQuery(new MatchAllDocsQuery(), new Script("test"), new ScoreScript.LeafFactory() {
@Override
public boolean needs_score() {
return false;
}

@Override
public ScoreScript newInstance(DocReader docReader) {
return new ScoreScript(Map.of(), searchContext.lookup(), docReader) {
@Override
public double execute(ExplanationHolder explanation) {
ScriptDocValues.Booleans booleans = (ScriptDocValues.Booleans) getDoc().get("test");
return booleans.get(0) ? 3 : 0;
public ScoreScript newInstance(DocReader docReader) {
return new ScoreScript(Map.of(), searchContext.lookup(), docReader) {
@Override
public double execute(ExplanationHolder explanation) {
ScriptDocValues.Booleans booleans = (ScriptDocValues.Booleans) getDoc().get("test");
return booleans.get(0) ? 3 : 0;
}
};
}
}, searchContext.lookup(), 2.5f, "test", 0, IndexVersion.current())),
equalTo(1)
);
}
{
SearchExecutionContext searchContext = mockContext(true, simpleMappedFieldType());
assertThat(
searcher.count(new ScriptScoreQuery(new MatchAllDocsQuery(), new Script("test"), new ScoreScript.LeafFactory() {
@Override
public boolean needs_score() {
return false;
}
};
}
}, searchContext.lookup(), 2.5f, "test", 0, IndexVersion.current())), equalTo(1));
assertThat(searcher.count(new ScriptScoreQuery(new MatchAllDocsQuery(), new Script("test"), new ScoreScript.LeafFactory() {
@Override
public boolean needs_score() {
return false;
}

@Override
public ScoreScript newInstance(DocReader docReader) {
return new ScoreScript(Map.of(), searchContext.lookup(), docReader) {
@Override
public double execute(ExplanationHolder explanation) {
BooleanDocValuesField booleans = (BooleanDocValuesField) field("test");
return booleans.getInternal(0) ? 3 : 0;
public ScoreScript newInstance(DocReader docReader) {
return new ScoreScript(Map.of(), searchContext.lookup(), docReader) {
@Override
public double execute(ExplanationHolder explanation) {
BooleanDocValuesField booleans = (BooleanDocValuesField) field("test");
return booleans.getInternal(0) ? 3 : 0;
}
};
}
};
}
}, searchContext.lookup(), 2.5f, "test", 0, IndexVersion.current())), equalTo(1));
}, searchContext.lookup(), 2.5f, "test", 0, IndexVersion.current())),
equalTo(1)
);
}
}
}
}
Expand Down
38 changes: 22 additions & 16 deletions server/src/test/java/org/elasticsearch/index/mapper/DateScriptFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,22 +247,28 @@ public void testDistanceFeatureQuery() throws IOException {
);
try (DirectoryReader reader = iw.getReader()) {
IndexSearcher searcher = newSearcher(reader);
Query query = simpleMappedFieldType().distanceFeatureQuery(1595432181354L, "1ms", mockContext());
TopDocs docs = searcher.search(query, 4);
assertThat(docs.scoreDocs, arrayWithSize(3));
assertThat(readSource(reader, docs.scoreDocs[0].doc), equalTo("{\"timestamp\": [1595432181354]}"));
assertThat(docs.scoreDocs[0].score, equalTo(1.0F));
assertThat(readSource(reader, docs.scoreDocs[1].doc), equalTo("{\"timestamp\": [1595432181356, 1]}"));
assertThat((double) docs.scoreDocs[1].score, closeTo(.333, .001));
assertThat(readSource(reader, docs.scoreDocs[2].doc), equalTo("{\"timestamp\": [1595432181351]}"));
assertThat((double) docs.scoreDocs[2].score, closeTo(.250, .001));
Explanation explanation = query.createWeight(searcher, ScoreMode.TOP_SCORES, 1.0F)
.explain(reader.leaves().get(0), docs.scoreDocs[0].doc);
assertThat(explanation.toString(), containsString("1.0 = Distance score, computed as weight * pivot / (pivot"));
assertThat(explanation.toString(), containsString("1.0 = weight"));
assertThat(explanation.toString(), containsString("1 = pivot"));
assertThat(explanation.toString(), containsString("1595432181354 = origin"));
assertThat(explanation.toString(), containsString("1595432181354 = current value"));
TopDocs docs;
{
Query query = simpleMappedFieldType().distanceFeatureQuery(1595432181354L, "1ms", mockContext());
docs = searcher.search(query, 4);
assertThat(docs.scoreDocs, arrayWithSize(3));
assertThat(readSource(reader, docs.scoreDocs[0].doc), equalTo("{\"timestamp\": [1595432181354]}"));
assertThat(docs.scoreDocs[0].score, equalTo(1.0F));
assertThat(readSource(reader, docs.scoreDocs[1].doc), equalTo("{\"timestamp\": [1595432181356, 1]}"));
assertThat((double) docs.scoreDocs[1].score, closeTo(.333, .001));
assertThat(readSource(reader, docs.scoreDocs[2].doc), equalTo("{\"timestamp\": [1595432181351]}"));
assertThat((double) docs.scoreDocs[2].score, closeTo(.250, .001));
}
{
Query query = simpleMappedFieldType().distanceFeatureQuery(1595432181354L, "1ms", mockContext());
Explanation explanation = query.createWeight(searcher, ScoreMode.TOP_SCORES, 1.0F)
.explain(reader.leaves().get(0), docs.scoreDocs[0].doc);
assertThat(explanation.toString(), containsString("1.0 = Distance score, computed as weight * pivot / (pivot"));
assertThat(explanation.toString(), containsString("1.0 = weight"));
assertThat(explanation.toString(), containsString("1 = pivot"));
assertThat(explanation.toString(), containsString("1595432181354 = origin"));
assertThat(explanation.toString(), containsString("1595432181354 = current value"));
}
}
}
}
Expand Down

0 comments on commit 83af00b

Please sign in to comment.