Skip to content

Commit

Permalink
Fix search response leaks in rank eval tests (elastic#105014)
Browse files Browse the repository at this point in the history
It's in the title. Somehow none of these tests were leak-proof.
Moving the responses to unpooled to fix things.

closes elastic#104570
  • Loading branch information
original-brownbear authored Feb 1, 2024
1 parent 6439614 commit 81f0c20
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void testDCGAt() {
SearchHit[] hits = new SearchHit[6];
for (int i = 0; i < 6; i++) {
rated.add(new RatedDocument("index", Integer.toString(i), relevanceRatings[i]));
hits[i] = new SearchHit(i, Integer.toString(i));
hits[i] = SearchHit.unpooled(i, Integer.toString(i));
hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
}
DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
Expand Down Expand Up @@ -111,7 +111,7 @@ public void testDCGAtSixMissingRatings() {
rated.add(new RatedDocument("index", Integer.toString(i), relevanceRatings[i]));
}
}
hits[i] = new SearchHit(i, Integer.toString(i));
hits[i] = SearchHit.unpooled(i, Integer.toString(i));
hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
}
DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
Expand Down Expand Up @@ -168,7 +168,7 @@ public void testDCGAtFourMoreRatings() {
// only create four hits
SearchHit[] hits = new SearchHit[4];
for (int i = 0; i < 4; i++) {
hits[i] = new SearchHit(i, Integer.toString(i));
hits[i] = SearchHit.unpooled(i, Integer.toString(i));
hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
}
DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private SearchHit[] createSearchHits(List<RatedDocument> rated, Integer[] releva
if (relevanceRatings[i] != null) {
rated.add(new RatedDocument("index", Integer.toString(i), relevanceRatings[i]));
}
hits[i] = new SearchHit(i, Integer.toString(i));
hits[i] = SearchHit.unpooled(i, Integer.toString(i));
hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
}
return hits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public void testXContentParsingIsNotLenient() throws IOException {
private static SearchHit[] createSearchHits(int from, int to, String index) {
SearchHit[] hits = new SearchHit[to + 1 - from];
for (int i = from; i <= to; i++) {
hits[i] = new SearchHit(i, i + "");
hits[i] = SearchHit.unpooled(i, i + "");
hits[i].shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
}
return hits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public void testIgnoreUnlabeled() {
rated.add(createRatedDoc("test", "1", RELEVANT_RATING));
// add an unlabeled search hit
SearchHit[] searchHits = Arrays.copyOf(toSearchHits(rated, "test"), 3);
searchHits[2] = new SearchHit(2, "2");
searchHits[2] = SearchHit.unpooled(2, "2");
searchHits[2].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));

EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", searchHits, rated);
Expand All @@ -120,7 +120,7 @@ public void testIgnoreUnlabeled() {
public void testNoRatedDocs() throws Exception {
SearchHit[] hits = new SearchHit[5];
for (int i = 0; i < 5; i++) {
hits[i] = new SearchHit(i, i + "");
hits[i] = SearchHit.unpooled(i, i + "");
hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
}
EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", hits, Collections.emptyList());
Expand Down Expand Up @@ -248,7 +248,7 @@ private static PrecisionAtK mutate(PrecisionAtK original) {
private static SearchHit[] toSearchHits(List<RatedDocument> rated, String index) {
SearchHit[] hits = new SearchHit[rated.size()];
for (int i = 0; i < rated.size(); i++) {
hits[i] = new SearchHit(i, i + "");
hits[i] = SearchHit.unpooled(i, i + "");
hits[i].shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
}
return hits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public void testToXContent() throws IOException {
}

private static RatedSearchHit searchHit(String index, int docId, Integer rating) {
SearchHit hit = new SearchHit(docId, docId + "");
SearchHit hit = SearchHit.unpooled(docId, docId + "");
hit.shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
hit.score(1.0f);
return new RatedSearchHit(hit, rating != null ? OptionalInt.of(rating) : OptionalInt.empty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.index.rankeval;

import org.apache.lucene.util.Constants;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -110,8 +109,6 @@ public static RatedRequest createTestItem(boolean forceRequest) {
}

public void testXContentRoundtrip() throws IOException {
assumeFalse("https://github.com/elastic/elasticsearch/issues/104570", Constants.WINDOWS);

RatedRequest testItem = createTestItem(randomBoolean());
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
XContentBuilder shuffled = shuffleXContent(testItem.toXContent(builder, ToXContent.EMPTY_PARAMS));
Expand All @@ -126,8 +123,6 @@ public void testXContentRoundtrip() throws IOException {
}

public void testXContentParsingIsNotLenient() throws IOException {
assumeFalse("https://github.com/elastic/elasticsearch/issues/104570", Constants.WINDOWS);

RatedRequest testItem = createTestItem(randomBoolean());
XContentType xContentType = randomFrom(XContentType.values());
BytesReference originalBytes = toShuffledXContent(testItem, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean());
Expand Down Expand Up @@ -266,8 +261,6 @@ public void testAggsNotAllowed() {
}

public void testSuggestionsNotAllowed() {
assumeFalse("https://github.com/elastic/elasticsearch/issues/104570", Constants.WINDOWS);

List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument("index1", "id1", 1));
SearchSourceBuilder query = new SearchSourceBuilder();
query.suggest(new SuggestBuilder().addSuggestion("id", SuggestBuilders.completionSuggestion("fieldname")));
Expand Down Expand Up @@ -306,8 +299,6 @@ public void testProfileNotAllowed() {
* matter for parsing xContent
*/
public void testParseFromXContent() throws IOException {
assumeFalse("https://github.com/elastic/elasticsearch/issues/104570", Constants.WINDOWS);

String querySpecString = """
{
"id": "my_qa_query",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class RatedSearchHitTests extends ESTestCase {

public static RatedSearchHit randomRatedSearchHit() {
OptionalInt rating = randomBoolean() ? OptionalInt.empty() : OptionalInt.of(randomIntBetween(0, 5));
SearchHit searchHit = new SearchHit(randomIntBetween(0, 10), randomAlphaOfLength(10));
SearchHit searchHit = SearchHit.unpooled(randomIntBetween(0, 10), randomAlphaOfLength(10));
RatedSearchHit ratedSearchHit = new RatedSearchHit(searchHit, rating);
return ratedSearchHit;
}
Expand All @@ -36,7 +36,7 @@ private static RatedSearchHit mutateTestItem(RatedSearchHit original) {
SearchHit hit = original.getSearchHit();
switch (randomIntBetween(0, 1)) {
case 0 -> rating = rating.isPresent() ? OptionalInt.of(rating.getAsInt() + 1) : OptionalInt.of(randomInt(5));
case 1 -> hit = new SearchHit(hit.docId(), hit.getId() + randomAlphaOfLength(10));
case 1 -> hit = SearchHit.unpooled(hit.docId(), hit.getId() + randomAlphaOfLength(10));
default -> throw new IllegalStateException("The test should only allow two parameters mutated");
}
return new RatedSearchHit(hit, rating);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void testNoRatedDocs() throws Exception {
int k = 5;
SearchHit[] hits = new SearchHit[k];
for (int i = 0; i < k; i++) {
hits[i] = new SearchHit(i, i + "");
hits[i] = SearchHit.unpooled(i, i + "");
hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
}

Expand Down Expand Up @@ -217,7 +217,7 @@ private static RecallAtK mutate(RecallAtK original) {
private static SearchHit[] toSearchHits(List<RatedDocument> rated, String index) {
SearchHit[] hits = new SearchHit[rated.size()];
for (int i = 0; i < rated.size(); i++) {
hits[i] = new SearchHit(i, i + "");
hits[i] = SearchHit.unpooled(i, i + "");
hits[i].shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
}
return hits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.index.rankeval;

import org.apache.lucene.util.Constants;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.search.MultiSearchRequest;
import org.elasticsearch.action.search.MultiSearchResponse;
Expand Down Expand Up @@ -43,8 +42,6 @@ public final class TransportRankEvalActionTests extends ESTestCase {
* Test that request parameters like indicesOptions or searchType from ranking evaluation request are transfered to msearch request
*/
public void testTransferRequestParameters() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/104570", Constants.WINDOWS);

String indexName = "test_index";
List<RatedRequest> specifications = new ArrayList<>();
specifications.add(
Expand Down

0 comments on commit 81f0c20

Please sign in to comment.