From 089690e46e22edb66bf086bf7c3c10dd1aa8e6a0 Mon Sep 17 00:00:00 2001 From: ChrisHegarty Date: Fri, 22 Sep 2023 16:15:57 +0100 Subject: [PATCH] Fix block hash test and revert mistaken changes --- .../blockhash/BytesRefLongBlockHash.java | 1 - .../aggregation/blockhash/IntBlockHash.java | 2 -- .../aggregation/blockhash/LongBlockHash.java | 3 ++- .../aggregation/blockhash/BlockHashTests.java | 15 +++++++++++++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/BytesRefLongBlockHash.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/BytesRefLongBlockHash.java index 0c5b60f471f8c..50fd1bb7b0943 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/BytesRefLongBlockHash.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/BytesRefLongBlockHash.java @@ -74,7 +74,6 @@ public void add(Page page, GroupingAggregatorFunction.AddInput addInput) { } else { new AddBlock(block1, block2, addInput).add(); } - Releasables.closeExpectNoException(block1, block2); } public IntVector add(BytesRefVector vector1, LongVector vector2) { diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/IntBlockHash.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/IntBlockHash.java index 7e5f3c94b91cb..4fcd9735f6158 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/IntBlockHash.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/IntBlockHash.java @@ -20,7 +20,6 @@ import org.elasticsearch.compute.data.Page; import org.elasticsearch.compute.operator.MultivalueDedupe; import org.elasticsearch.compute.operator.MultivalueDedupeInt; -import org.elasticsearch.core.Releasables; import java.util.BitSet; @@ -53,7 +52,6 @@ public void add(Page page, GroupingAggregatorFunction.AddInput addInput) { } else { addInput.add(0, add(vector)); } - Releasables.closeExpectNoException(block); } private IntVector add(IntVector vector) { diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongBlockHash.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongBlockHash.java index b8b66e2197b63..5e5b46ae6eda1 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongBlockHash.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongBlockHash.java @@ -13,6 +13,7 @@ import org.elasticsearch.compute.aggregation.GroupingAggregatorFunction; import org.elasticsearch.compute.aggregation.SeenGroupIds; import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.IntArrayVector; import org.elasticsearch.compute.data.IntBlock; import org.elasticsearch.compute.data.IntVector; import org.elasticsearch.compute.data.LongArrayBlock; @@ -62,7 +63,7 @@ private IntVector add(LongVector vector) { for (int i = 0; i < vector.getPositionCount(); i++) { groups[i] = Math.toIntExact(hashOrdToGroupNullReserved(longHash.add(vector.getLong(i)))); } - return vector.blockFactory().newIntArrayVector(groups, groups.length); + return new IntArrayVector(groups, groups.length); } private IntBlock add(LongBlock block) { diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/blockhash/BlockHashTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/blockhash/BlockHashTests.java index 620bb5ab5319a..d6345c6c0a453 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/blockhash/BlockHashTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/blockhash/BlockHashTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.compute.data.Page; import org.elasticsearch.compute.operator.HashAggregationOperator; import org.elasticsearch.core.Releasables; +import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.test.ESTestCase; import org.junit.After; @@ -46,11 +47,14 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class BlockHashTests extends ESTestCase { - static final CircuitBreaker breaker = new MockBigArrays.LimitedBreaker("esql-test-breaker", ByteSizeValue.ofGb(1)); - static final BlockFactory blockFactory = BlockFactory.getInstance(breaker, BigArrays.NON_RECYCLING_INSTANCE); + final CircuitBreaker breaker = new MockBigArrays.LimitedBreaker("esql-test-breaker", ByteSizeValue.ofGb(1)); + final BigArrays bigArrays = new MockBigArrays(PageCacheRecycler.NON_RECYCLING_INSTANCE, mockBreakerService(breaker)); + final BlockFactory blockFactory = BlockFactory.getInstance(breaker, bigArrays); @ParametersFactory public static List params() { @@ -1189,4 +1193,11 @@ private void assertKeys(Block[] actualKeys, Object[][] expectedKeys) { } } } + + // A breaker service that always returns the given breaker for getBreaker(CircuitBreaker.REQUEST) + static CircuitBreakerService mockBreakerService(CircuitBreaker breaker) { + CircuitBreakerService breakerService = mock(CircuitBreakerService.class); + when(breakerService.getBreaker(CircuitBreaker.REQUEST)).thenReturn(breaker); + return breakerService; + } }