Fix leak in collapsing search results (elastic#110927)

Fixing this case for now by enforcing unpooled to plug the leak, this needs a little more work to function well pooled.
lkts · Jul 16, 2024 · e317430 · e317430
1 parent eac7604
commit e317430
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 0 deletions.
diff --git a/docs/changelog/110927.yaml b/docs/changelog/110927.yaml
@@ -0,0 +1,5 @@
+pr: 110927
+summary: Fix leak in collapsing search results
+area: Search
+type: bug
+issues: []
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java
@@ -61,4 +61,27 @@ public void testCollapseWithDocValueFields() {
             }
         );
     }
+
+    public void testCollapseWithFields() {
+        final String indexName = "test_collapse";
+        createIndex(indexName);
+        final String collapseField = "collapse_field";
+        final String otherField = "other_field";
+        assertAcked(indicesAdmin().preparePutMapping(indexName).setSource(collapseField, "type=keyword", otherField, "type=keyword"));
+        index(indexName, "id_1_0", Map.of(collapseField, "value1", otherField, "other_value1"));
+        index(indexName, "id_1_1", Map.of(collapseField, "value1", otherField, "other_value2"));
+        index(indexName, "id_2_0", Map.of(collapseField, "value2", otherField, "other_value3"));
+        refresh(indexName);
+
+        assertNoFailuresAndResponse(
+            prepareSearch(indexName).setQuery(new MatchAllQueryBuilder())
+                .setFetchSource(false)
+                .addFetchField(otherField)
+                .setCollapse(new CollapseBuilder(collapseField).setInnerHits(new InnerHitBuilder("ih").setSize(2))),
+            searchResponse -> {
+                assertEquals(collapseField, searchResponse.getHits().getCollapseField());
+                assertEquals(Set.of(new BytesRef("value1"), new BytesRef("value2")), Set.of(searchResponse.getHits().getCollapseValues()));
+            }
+        );
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java
@@ -100,6 +100,10 @@ private void doRun() {
                     if (hit.getInnerHits() == null) {
                         hit.setInnerHits(Maps.newMapWithExpectedSize(innerHitBuilders.size()));
                     }
+                    if (hit.isPooled() == false) {
+                        // TODO: make this work pooled by forcing the hit itself to become pooled as needed here
+                        innerHits = innerHits.asUnpooled();
+                    }
                     hit.getInnerHits().put(innerHitBuilder.getName(), innerHits);
                     assert innerHits.isPooled() == false || hit.isPooled() : "pooled inner hits can only be added to a pooled hit";
                     innerHits.mustIncRef();