diff --git a/x-pack/plugin/esql/compute/src/main/java/module-info.java b/x-pack/plugin/esql/compute/src/main/java/module-info.java index 3104fb05280eb..195c5fff6142b 100644 --- a/x-pack/plugin/esql/compute/src/main/java/module-info.java +++ b/x-pack/plugin/esql/compute/src/main/java/module-info.java @@ -6,7 +6,6 @@ */ module org.elasticsearch.compute { - uses org.elasticsearch.compute.data.BlockFactoryParameters; requires org.apache.lucene.core; requires org.elasticsearch.base; diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactory.java index 63de604c49b18..780155f8a8407 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactory.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactory.java @@ -16,8 +16,6 @@ import org.elasticsearch.compute.data.Block.MvOrdering; import java.util.BitSet; -import java.util.List; -import java.util.ServiceLoader; public class BlockFactory { @@ -26,22 +24,6 @@ public class BlockFactory { BigArrays.NON_RECYCLING_INSTANCE ); - private static final BlockFactory GLOBAL = loadGlobalFactory(); - // new BlockFactory(new NoopCircuitBreaker("esql_noop_breaker"), BigArrays.NON_RECYCLING_INSTANCE); - - private static BlockFactory loadGlobalFactory() { - ServiceLoader loader = ServiceLoader.load( - BlockFactoryParameters.class, - BlockFactory.class.getClassLoader() - ); - List> impls = loader.stream().toList(); - if (impls.size() != 1) { - throw new AssertionError("expected exactly one impl, but got:" + impls); - } - BlockFactoryParameters params = impls.get(0).get(); - return new BlockFactory(params.breaker(), params.bigArrays()); - } - private final CircuitBreaker breaker; private final BigArrays bigArrays; @@ -51,13 +33,6 @@ public BlockFactory(CircuitBreaker breaker, BigArrays bigArrays) { this.bigArrays = bigArrays; } - /** - * Returns the global ESQL block factory. - */ - public static BlockFactory getGlobalInstance() { - return GLOBAL; - } - /** * Returns the Non-Breaking block factory. */ diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactoryParameters.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactoryParameters.java deleted file mode 100644 index a9dc11635f8c0..0000000000000 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockFactoryParameters.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.compute.data; - -import org.elasticsearch.common.breaker.CircuitBreaker; -import org.elasticsearch.common.util.BigArrays; - -/** - * Allows to inject instances of a breaker and bigArrays into the Global block factory. - * The Global factory is somewhat temporary, therefore this interface and its ServiceLoader - * machinery can be removed once the Global factory is removed. - */ -public interface BlockFactoryParameters { - - CircuitBreaker breaker(); - - BigArrays bigArrays(); -} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/TestBlockFactoryParameters.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/TestBlockFactoryParameters.java deleted file mode 100644 index 8fa38b6864674..0000000000000 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/TestBlockFactoryParameters.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.compute; - -import org.elasticsearch.common.breaker.CircuitBreaker; -import org.elasticsearch.common.unit.ByteSizeValue; -import org.elasticsearch.common.util.BigArrays; -import org.elasticsearch.common.util.MockBigArrays; -import org.elasticsearch.common.util.PageCacheRecycler; -import org.elasticsearch.compute.data.BlockFactoryParameters; -import org.elasticsearch.indices.breaker.CircuitBreakerService; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class TestBlockFactoryParameters implements BlockFactoryParameters { - - final CircuitBreaker breaker; - final BigArrays bigArrays; - - public TestBlockFactoryParameters() { - breaker = new MockBigArrays.LimitedBreaker("esql-test-breaker", ByteSizeValue.ofGb(1)); - var breakerService = mock(CircuitBreakerService.class); - when(breakerService.getBreaker(CircuitBreaker.REQUEST)).thenReturn(breaker); - bigArrays = new MockBigArrays(PageCacheRecycler.NON_RECYCLING_INSTANCE, breakerService); - } - - @Override - public CircuitBreaker breaker() { - return breaker; - } - - @Override - public BigArrays bigArrays() { - return bigArrays; - } -} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockFactoryTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockFactoryTests.java index 7be79e73b5d9d..9c6c9d966b3f6 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockFactoryTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockFactoryTests.java @@ -61,16 +61,6 @@ public BlockFactory get() { public String toString() { return "1gb"; } - }, new Supplier<>() { - @Override - public BlockFactory get() { - return BlockFactory.getGlobalInstance(); - } - - @Override - public String toString() { - return "global"; - } }); return l.stream().map(s -> new Object[] { s }).toList(); } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java index 2636a7cf3c133..19893bf8072f1 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlDisruptionIT.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.action; -import org.apache.lucene.tests.util.LuceneTestCase; import org.elasticsearch.action.ActionFuture; import org.elasticsearch.cluster.coordination.Coordinator; import org.elasticsearch.cluster.coordination.FollowersChecker; @@ -19,7 +18,6 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.disruption.NetworkDisruption; import org.elasticsearch.test.disruption.ServiceDisruptionScheme; -import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.transport.TransportSettings; @@ -31,9 +29,7 @@ import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -@TestLogging(value = "org.elasticsearch.indices.breaker:TRACE", reason = "failing") @ESIntegTestCase.ClusterScope(scope = TEST, minNumDataNodes = 2, maxNumDataNodes = 4) -@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/99173") public class EsqlDisruptionIT extends EsqlActionIT { // copied from AbstractDisruptionTestCase diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/lookup/EnrichLookupIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/lookup/EnrichLookupIT.java index fa5a1617e9d61..f9d97cbd910e0 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/lookup/EnrichLookupIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/lookup/EnrichLookupIT.java @@ -232,7 +232,7 @@ public void testMultipleMatches() { static DriverContext driverContext() { return new DriverContext( new MockBigArrays(PageCacheRecycler.NON_RECYCLING_INSTANCE, new NoneCircuitBreakerService()).withCircuitBreaking(), - BlockFactory.getGlobalInstance() + BlockFactory.getNonBreakingInstance() ); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlBlockFactoryParams.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlBlockFactoryParams.java deleted file mode 100644 index 1ca1d5e217f6a..0000000000000 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlBlockFactoryParams.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.esql.plugin; - -import org.elasticsearch.common.breaker.CircuitBreaker; -import org.elasticsearch.common.breaker.NoopCircuitBreaker; -import org.elasticsearch.common.util.BigArrays; -import org.elasticsearch.compute.data.BlockFactoryParameters; - -/** A provider for sharing the given parameters with the compute engine's block factory. */ -public class EsqlBlockFactoryParams implements BlockFactoryParameters { - - static final CircuitBreaker NOOP_BREAKER = new NoopCircuitBreaker("ESQL-noop-breaker"); - - static CircuitBreaker ESQL_BREAKER; - static BigArrays ESQL_BIGARRAYS; - - static void init(BigArrays bigArrays) { - ESQL_BREAKER = bigArrays.breakerService().getBreaker("request"); - ESQL_BIGARRAYS = bigArrays; - } - - final CircuitBreaker breaker; - final BigArrays bigArrays; - - public EsqlBlockFactoryParams() { - this.breaker = ESQL_BREAKER; - this.bigArrays = ESQL_BIGARRAYS; - } - - @Override - public CircuitBreaker breaker() { - return breaker != null ? breaker : NOOP_BREAKER; - } - - @Override - public BigArrays bigArrays() { - return bigArrays != null ? bigArrays : BigArrays.NON_RECYCLING_INSTANCE; - } -} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java index 1ff00401029cf..b9ab0f7646b96 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java @@ -12,6 +12,7 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; @@ -37,6 +38,7 @@ import java.time.ZoneOffset; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.concurrent.Executor; public class TransportEsqlQueryAction extends HandledTransportAction { @@ -69,8 +71,7 @@ public TransportEsqlQueryAction( this.requestExecutor = threadPool.executor(EsqlPlugin.ESQL_THREAD_POOL_NAME); exchangeService.registerTransportHandler(transportService); this.exchangeService = exchangeService; - EsqlBlockFactoryParams.init(bigArrays); - var blockFactory = BlockFactory.getGlobalInstance(); + var blockFactory = createBlockFactory(bigArrays); this.enrichPolicyResolver = new EnrichPolicyResolver(clusterService, transportService, planExecutor.indexResolver()); this.enrichLookupService = new EnrichLookupService(clusterService, searchService, transportService, bigArrays, blockFactory); this.computeService = new ComputeService( @@ -85,6 +86,12 @@ public TransportEsqlQueryAction( this.settings = settings; } + static BlockFactory createBlockFactory(BigArrays bigArrays) { + CircuitBreaker circuitBreaker = bigArrays.breakerService().getBreaker("request"); + Objects.requireNonNull(circuitBreaker, "request circuit breaker wasn't set"); + return new BlockFactory(circuitBreaker, bigArrays); + } + @Override protected void doExecute(Task task, EsqlQueryRequest request, ActionListener listener) { // workaround for https://github.com/elastic/elasticsearch/issues/97916 - TODO remove this when we can diff --git a/x-pack/plugin/esql/src/main/resources/META-INF/services/org.elasticsearch.compute.data.BlockFactoryParameters b/x-pack/plugin/esql/src/main/resources/META-INF/services/org.elasticsearch.compute.data.BlockFactoryParameters deleted file mode 100644 index e397954c84cbe..0000000000000 --- a/x-pack/plugin/esql/src/main/resources/META-INF/services/org.elasticsearch.compute.data.BlockFactoryParameters +++ /dev/null @@ -1,8 +0,0 @@ -# -# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -# or more contributor license agreements. Licensed under the Elastic License -# 2.0; you may not use this file except in compliance with the Elastic License -# 2.0. -# - -org.elasticsearch.xpack.esql.plugin.EsqlBlockFactoryParams diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 8a5b021addae5..a3b63e7e34c37 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -327,7 +327,7 @@ private ActualResults executePlan() throws Exception { sessionId, new CancellableTask(1, "transport", "esql", null, TaskId.EMPTY_TASK_ID, Map.of()), bigArrays, - BlockFactory.getGlobalInstance(), + BlockFactory.getNonBreakingInstance(), configuration, exchangeSource, exchangeSink, diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java index 995eed164b144..6633e5ae3c0fe 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java @@ -637,8 +637,9 @@ private static void writeToTempDir(String subdir, String str, String extension) */ protected DriverContext driverContext() { MockBigArrays bigArrays = new MockBigArrays(PageCacheRecycler.NON_RECYCLING_INSTANCE, ByteSizeValue.ofGb(1)); - breakers.add(bigArrays.breakerService().getBreaker(CircuitBreaker.REQUEST)); - return new DriverContext(bigArrays.withCircuitBreaking(), BlockFactory.getGlobalInstance()); + CircuitBreaker breaker = bigArrays.breakerService().getBreaker(CircuitBreaker.REQUEST); + breakers.add(breaker); + return new DriverContext(bigArrays.withCircuitBreaking(), new BlockFactory(breaker, bigArrays)); } @After diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/EvalMapperTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/EvalMapperTests.java index f0d4f0534caee..904e2c2ce7e8f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/EvalMapperTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/EvalMapperTests.java @@ -159,7 +159,7 @@ private static FieldAttribute field(String name, DataType type) { static DriverContext driverContext() { return new DriverContext( new MockBigArrays(PageCacheRecycler.NON_RECYCLING_INSTANCE, new NoneCircuitBreakerService()).withCircuitBreaking(), - BlockFactory.getGlobalInstance() + BlockFactory.getNonBreakingInstance() ); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlannerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlannerTests.java index 95ef6e7baf70c..f66aa9f47cb8d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlannerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlannerTests.java @@ -119,7 +119,7 @@ private LocalExecutionPlanner planner() throws IOException { "test", null, BigArrays.NON_RECYCLING_INSTANCE, - BlockFactory.getGlobalInstance(), + BlockFactory.getNonBreakingInstance(), config(), null, null,