From ba7b2ffc2ae9770373578cfe78bd49d046025aa6 Mon Sep 17 00:00:00 2001 From: penghuo Date: Tue, 13 Sep 2022 08:51:18 -0700 Subject: [PATCH] Change ApplicationContext lifecycle to per node level Signed-off-by: penghuo --- .../expression/config/ExpressionConfig.java | 4 ++ .../sql/legacy/plugin/RestSQLQueryAction.java | 47 ++------------- .../sql/legacy/plugin/RestSqlAction.java | 15 +++-- .../legacy/plugin/RestSQLQueryActionTest.java | 31 +++++----- .../opensearch/security/SecurityAccess.java | 6 +- .../org/opensearch/sql/plugin/SQLPlugin.java | 48 +++++++++------ .../plugin/catalog/CatalogServiceImpl.java | 13 +---- .../plugin/config/OpenSearchPluginConfig.java | 19 ++++-- .../plugin/rest/OpenSearchPluginConfig.java | 58 ------------------- .../sql/plugin/rest/RestPPLQueryAction.java | 1 - .../transport/TransportPPLQueryAction.java | 37 ++---------- ppl/build.gradle | 1 - .../sql/ppl/config/PPLServiceConfig.java | 3 + .../sql/sql/config/SQLServiceConfig.java | 14 ++--- 14 files changed, 99 insertions(+), 198 deletions(-) rename legacy/src/main/java/org/opensearch/sql/legacy/plugin/OpenSearchSQLPluginConfig.java => plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginConfig.java (73%) delete mode 100644 plugin/src/main/java/org/opensearch/sql/plugin/rest/OpenSearchPluginConfig.java diff --git a/core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java b/core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java index 76e0eb0326..886d7ae399 100644 --- a/core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java +++ b/core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java @@ -20,8 +20,10 @@ import org.opensearch.sql.expression.operator.predicate.UnaryPredicateOperator; import org.opensearch.sql.expression.text.TextFunction; import org.opensearch.sql.expression.window.WindowFunctions; +import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Scope; /** * Expression Config for Spring IoC. @@ -32,6 +34,7 @@ public class ExpressionConfig { * BuiltinFunctionRepository constructor. */ @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public BuiltinFunctionRepository functionRepository() { BuiltinFunctionRepository builtinFunctionRepository = new BuiltinFunctionRepository(new HashMap<>()); @@ -50,6 +53,7 @@ public BuiltinFunctionRepository functionRepository() { } @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public DSL dsl(BuiltinFunctionRepository repository) { return new DSL(repository); } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java index 0db08398b8..6524ac2f0e 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java @@ -11,23 +11,17 @@ import static org.opensearch.sql.executor.ExecutionEngine.QueryResponse; import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; -import java.io.IOException; -import java.security.PrivilegedExceptionAction; import java.util.List; -import javax.xml.catalog.Catalog; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.client.node.NodeClient; -import org.opensearch.cluster.service.ClusterService; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; -import org.opensearch.sql.catalog.CatalogService; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.response.ResponseListener; -import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.executor.ExecutionEngine.ExplainResponse; import org.opensearch.sql.legacy.metrics.MetricName; import org.opensearch.sql.legacy.metrics.Metrics; @@ -41,7 +35,6 @@ import org.opensearch.sql.protocol.response.format.RawResponseFormatter; import org.opensearch.sql.protocol.response.format.ResponseFormatter; import org.opensearch.sql.sql.SQLService; -import org.opensearch.sql.sql.config.SQLServiceConfig; import org.opensearch.sql.sql.domain.SQLQueryRequest; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -56,23 +49,14 @@ public class RestSQLQueryAction extends BaseRestHandler { public static final RestChannelConsumer NOT_SUPPORTED_YET = null; - private final ClusterService clusterService; - - /** - * Settings required by been initialization. - */ - private final Settings pluginSettings; - - private final CatalogService catalogService; + private final AnnotationConfigApplicationContext applicationContext; /** * Constructor of RestSQLQueryAction. */ - public RestSQLQueryAction(ClusterService clusterService, Settings pluginSettings, CatalogService catalogService) { + public RestSQLQueryAction(AnnotationConfigApplicationContext applicationContext) { super(); - this.clusterService = clusterService; - this.pluginSettings = pluginSettings; - this.catalogService = catalogService; + this.applicationContext = applicationContext; } @Override @@ -101,7 +85,8 @@ public RestChannelConsumer prepareRequest(SQLQueryRequest request, NodeClient no return NOT_SUPPORTED_YET; } - SQLService sqlService = createSQLService(nodeClient); + SQLService sqlService = + SecurityAccess.doPrivileged(() -> applicationContext.getBean(SQLService.class)); PhysicalPlan plan; try { // For now analyzing and planning stage may throw syntax exception as well @@ -123,20 +108,6 @@ public RestChannelConsumer prepareRequest(SQLQueryRequest request, NodeClient no return channel -> sqlService.execute(plan, createQueryResponseListener(channel, request)); } - private SQLService createSQLService(NodeClient client) { - return doPrivileged(() -> { - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); - context.registerBean(ClusterService.class, () -> clusterService); - context.registerBean(NodeClient.class, () -> client); - context.registerBean(Settings.class, () -> pluginSettings); - context.registerBean(CatalogService.class, () -> catalogService); - context.register(OpenSearchSQLPluginConfig.class); - context.register(SQLServiceConfig.class); - context.refresh(); - return context.getBean(SQLService.class); - }); - } - private ResponseListener createExplainResponseListener(RestChannel channel) { return new ResponseListener() { @Override @@ -185,14 +156,6 @@ public void onFailure(Exception e) { }; } - private T doPrivileged(PrivilegedExceptionAction action) { - try { - return SecurityAccess.doPrivileged(action); - } catch (IOException e) { - throw new IllegalStateException("Failed to perform privileged action", e); - } - } - private void sendResponse(RestChannel channel, RestStatus status, String content) { channel.sendResponse(new BytesRestResponse( status, "application/json; charset=UTF-8", content)); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java index ab146404f8..d47dac9325 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java @@ -27,7 +27,6 @@ import org.apache.logging.log4j.Logger; import org.opensearch.client.Client; import org.opensearch.client.node.NodeClient; -import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.index.IndexNotFoundException; import org.opensearch.rest.BaseRestHandler; @@ -35,7 +34,6 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; -import org.opensearch.sql.catalog.CatalogService; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.exception.ExpressionEvaluationException; @@ -65,6 +63,7 @@ import org.opensearch.sql.legacy.utils.JsonPrettyFormatter; import org.opensearch.sql.legacy.utils.QueryDataAnonymizer; import org.opensearch.sql.sql.domain.SQLQueryRequest; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; public class RestSqlAction extends BaseRestHandler { @@ -89,12 +88,16 @@ public class RestSqlAction extends BaseRestHandler { */ private final RestSQLQueryAction newSqlQueryHandler; - public RestSqlAction(Settings settings, ClusterService clusterService, - org.opensearch.sql.common.setting.Settings pluginSettings, - CatalogService catalogService) { + /** + * Application context used to create SQLService for each request. + */ + private final AnnotationConfigApplicationContext applicationContext; + + public RestSqlAction(Settings settings, AnnotationConfigApplicationContext applicationContext) { super(); this.allowExplicitIndex = MULTI_ALLOW_EXPLICIT_INDEX.get(settings); - this.newSqlQueryHandler = new RestSQLQueryAction(clusterService, pluginSettings, catalogService); + this.newSqlQueryHandler = new RestSQLQueryAction(applicationContext); + this.applicationContext = applicationContext; } @Override diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/plugin/RestSQLQueryActionTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/plugin/RestSQLQueryActionTest.java index 56d153eb9d..6257f0dd95 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/plugin/RestSQLQueryActionTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/plugin/RestSQLQueryActionTest.java @@ -8,7 +8,6 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; -import static org.mockito.Mockito.when; import static org.opensearch.sql.legacy.plugin.RestSQLQueryAction.NOT_SUPPORTED_YET; import static org.opensearch.sql.legacy.plugin.RestSqlAction.EXPLAIN_API_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT; @@ -18,36 +17,38 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.opensearch.client.node.NodeClient; -import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.sql.catalog.CatalogService; -import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.executor.ExecutionEngine; +import org.opensearch.sql.sql.config.SQLServiceConfig; import org.opensearch.sql.sql.domain.SQLQueryRequest; +import org.opensearch.sql.storage.StorageEngine; import org.opensearch.threadpool.ThreadPool; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; @RunWith(MockitoJUnitRunner.class) public class RestSQLQueryActionTest { - @Mock - private ClusterService clusterService; - private NodeClient nodeClient; @Mock private ThreadPool threadPool; - @Mock - private Settings settings; - - @Mock - private CatalogService catalogService; + private AnnotationConfigApplicationContext context; @Before public void setup() { nodeClient = new NodeClient(org.opensearch.common.settings.Settings.EMPTY, threadPool); - when(threadPool.getThreadContext()) + context = new AnnotationConfigApplicationContext(); + context.registerBean(StorageEngine.class, () -> Mockito.mock(StorageEngine.class)); + context.registerBean(ExecutionEngine.class, () -> Mockito.mock(ExecutionEngine.class)); + context.registerBean(CatalogService.class, () -> Mockito.mock(CatalogService.class)); + context.register(SQLServiceConfig.class); + context.refresh(); + Mockito.lenient().when(threadPool.getThreadContext()) .thenReturn(new ThreadContext(org.opensearch.common.settings.Settings.EMPTY)); } @@ -59,7 +60,7 @@ public void handleQueryThatCanSupport() { QUERY_API_ENDPOINT, ""); - RestSQLQueryAction queryAction = new RestSQLQueryAction(clusterService, settings, catalogService); + RestSQLQueryAction queryAction = new RestSQLQueryAction(context); assertNotSame(NOT_SUPPORTED_YET, queryAction.prepareRequest(request, nodeClient)); } @@ -71,7 +72,7 @@ public void handleExplainThatCanSupport() { EXPLAIN_API_ENDPOINT, ""); - RestSQLQueryAction queryAction = new RestSQLQueryAction(clusterService, settings, catalogService); + RestSQLQueryAction queryAction = new RestSQLQueryAction(context); assertNotSame(NOT_SUPPORTED_YET, queryAction.prepareRequest(request, nodeClient)); } @@ -84,7 +85,7 @@ public void skipQueryThatNotSupport() { QUERY_API_ENDPOINT, ""); - RestSQLQueryAction queryAction = new RestSQLQueryAction(clusterService, settings, catalogService); + RestSQLQueryAction queryAction = new RestSQLQueryAction(context); assertSame(NOT_SUPPORTED_YET, queryAction.prepareRequest(request, nodeClient)); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/security/SecurityAccess.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/security/SecurityAccess.java index cff219578f..0c1b2e58b1 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/security/SecurityAccess.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/security/SecurityAccess.java @@ -6,7 +6,6 @@ package org.opensearch.sql.opensearch.security; -import java.io.IOException; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; @@ -21,13 +20,12 @@ public class SecurityAccess { /** * Execute the operation in privileged mode. */ - public static T doPrivileged(final PrivilegedExceptionAction operation) - throws IOException { + public static T doPrivileged(final PrivilegedExceptionAction operation) { SpecialPermission.check(); try { return AccessController.doPrivileged(operation); } catch (final PrivilegedActionException e) { - throw (IOException) e.getCause(); + throw new IllegalStateException("Failed to perform privileged action", e); } } } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index 200364580b..a53c52eb41 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -40,30 +40,35 @@ import org.opensearch.script.ScriptContext; import org.opensearch.script.ScriptEngine; import org.opensearch.script.ScriptService; +import org.opensearch.sql.catalog.CatalogService; import org.opensearch.sql.legacy.esdomain.LocalClusterState; import org.opensearch.sql.legacy.executor.AsyncRestExecutor; import org.opensearch.sql.legacy.metrics.Metrics; import org.opensearch.sql.legacy.plugin.RestSqlAction; import org.opensearch.sql.legacy.plugin.RestSqlStatsAction; import org.opensearch.sql.opensearch.client.OpenSearchNodeClient; +import org.opensearch.sql.opensearch.security.SecurityAccess; import org.opensearch.sql.opensearch.setting.LegacyOpenDistroSettings; import org.opensearch.sql.opensearch.setting.OpenSearchSettings; import org.opensearch.sql.opensearch.storage.OpenSearchStorageEngine; import org.opensearch.sql.opensearch.storage.script.ExpressionScriptEngine; import org.opensearch.sql.opensearch.storage.serialization.DefaultExpressionSerializer; import org.opensearch.sql.plugin.catalog.CatalogServiceImpl; -import org.opensearch.sql.plugin.catalog.CatalogSettings; +import org.opensearch.sql.plugin.config.OpenSearchPluginConfig; import org.opensearch.sql.plugin.rest.RestPPLQueryAction; import org.opensearch.sql.plugin.rest.RestPPLStatsAction; import org.opensearch.sql.plugin.rest.RestQuerySettingsAction; import org.opensearch.sql.plugin.transport.PPLQueryAction; import org.opensearch.sql.plugin.transport.TransportPPLQueryAction; import org.opensearch.sql.plugin.transport.TransportPPLQueryResponse; +import org.opensearch.sql.ppl.config.PPLServiceConfig; +import org.opensearch.sql.sql.config.SQLServiceConfig; import org.opensearch.sql.storage.StorageEngine; import org.opensearch.threadpool.ExecutorBuilder; import org.opensearch.threadpool.FixedExecutorBuilder; import org.opensearch.threadpool.ThreadPool; import org.opensearch.watcher.ResourceWatcherService; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; public class SQLPlugin extends Plugin implements ActionPlugin, ScriptPlugin, ReloadablePlugin { @@ -76,6 +81,8 @@ public class SQLPlugin extends Plugin implements ActionPlugin, ScriptPlugin, Rel private NodeClient client; + private AnnotationConfigApplicationContext applicationContext; + public String name() { return "sql"; } @@ -101,8 +108,7 @@ public List getRestHandlers( return Arrays.asList( new RestPPLQueryAction(pluginSettings, settings), - new RestSqlAction(settings, clusterService, pluginSettings, - CatalogServiceImpl.getInstance()), + new RestSqlAction(settings, applicationContext), new RestSqlStatsAction(settings, restController), new RestPPLStatsAction(settings, restController), new RestQuerySettingsAction(settings, restController)); @@ -137,21 +143,29 @@ public Collection createComponents( this.client = (NodeClient) client; CatalogServiceImpl.getInstance().loadConnectors(clusterService.getSettings()); CatalogServiceImpl.getInstance().registerOpenSearchStorageEngine(openSearchStorageEngine()); + + this.applicationContext = new AnnotationConfigApplicationContext(); + SecurityAccess.doPrivileged( + () -> { + applicationContext.registerBean(ClusterService.class, () -> clusterService); + applicationContext.registerBean(NodeClient.class, () -> (NodeClient) client); + applicationContext.registerBean( + org.opensearch.sql.common.setting.Settings.class, () -> pluginSettings); + applicationContext.registerBean( + CatalogService.class, () -> CatalogServiceImpl.getInstance()); + applicationContext.register(OpenSearchPluginConfig.class); + applicationContext.register(PPLServiceConfig.class); + applicationContext.register(SQLServiceConfig.class); + applicationContext.refresh(); + return null; + }); + LocalClusterState.state().setClusterService(clusterService); LocalClusterState.state().setPluginSettings((OpenSearchSettings) pluginSettings); - return super.createComponents( - client, - clusterService, - threadPool, - resourceWatcherService, - scriptService, - contentRegistry, - environment, - nodeEnvironment, - namedWriteableRegistry, - indexNameResolver, - repositoriesServiceSupplier); + // return objects used by Guice to inject dependencies for e.g., + // transport action handler constructors + return ImmutableList.of(applicationContext); } @Override @@ -170,7 +184,6 @@ public List> getSettings() { return new ImmutableList.Builder>() .addAll(LegacyOpenDistroSettings.legacySettings()) .addAll(OpenSearchSettings.pluginSettings()) - .add(CatalogSettings.CATALOG_CONFIG) .build(); } @@ -186,8 +199,7 @@ public void reload(Settings settings) { } private StorageEngine openSearchStorageEngine() { - return new OpenSearchStorageEngine(new OpenSearchNodeClient(client), - pluginSettings); + return new OpenSearchStorageEngine(new OpenSearchNodeClient(client), pluginSettings); } } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java b/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java index 5a77961d8b..4a922e65ca 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java @@ -11,7 +11,6 @@ import java.io.IOException; import java.io.InputStream; import java.net.URISyntaxException; -import java.security.PrivilegedExceptionAction; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -54,7 +53,7 @@ private CatalogServiceImpl() { * @param settings settings. */ public void loadConnectors(Settings settings) { - doPrivileged(() -> { + SecurityAccess.doPrivileged(() -> { InputStream inputStream = CatalogSettings.CATALOG_CONFIG.get(settings); if (inputStream != null) { ObjectMapper objectMapper = new ObjectMapper(); @@ -96,14 +95,6 @@ public void registerOpenSearchStorageEngine(StorageEngine storageEngine) { storageEngineMap.put(OPEN_SEARCH, storageEngine); } - private T doPrivileged(PrivilegedExceptionAction action) { - try { - return SecurityAccess.doPrivileged(action); - } catch (IOException e) { - throw new IllegalStateException("Failed to perform privileged action", e); - } - } - private StorageEngine createStorageEngine(CatalogMetadata catalog) throws URISyntaxException { StorageEngine storageEngine; ConnectorType connector = catalog.getConnector(); @@ -165,4 +156,4 @@ private void validateCatalogs(List catalogs) { } -} \ No newline at end of file +} diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/OpenSearchSQLPluginConfig.java b/plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginConfig.java similarity index 73% rename from legacy/src/main/java/org/opensearch/sql/legacy/plugin/OpenSearchSQLPluginConfig.java rename to plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginConfig.java index b396d896b0..a2169eb839 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/OpenSearchSQLPluginConfig.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginConfig.java @@ -1,10 +1,13 @@ /* - * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. */ -package org.opensearch.sql.legacy.plugin; +package org.opensearch.sql.plugin.config; import org.opensearch.client.node.NodeClient; import org.opensearch.sql.common.setting.Settings; @@ -23,16 +26,19 @@ import org.opensearch.sql.opensearch.storage.OpenSearchStorageEngine; import org.opensearch.sql.storage.StorageEngine; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Scope; /** - * OpenSearch Plugin Config for SQL. + * OpenSearch plugin config that injects cluster service and node client from plugin + * and initialize OpenSearch storage and execution engine. */ @Configuration @Import({ExpressionConfig.class}) -public class OpenSearchSQLPluginConfig { +public class OpenSearchPluginConfig { @Autowired private NodeClient nodeClient; @@ -44,27 +50,32 @@ public class OpenSearchSQLPluginConfig { private BuiltinFunctionRepository functionRepository; @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public OpenSearchClient client() { return new OpenSearchNodeClient(nodeClient); } @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public StorageEngine storageEngine() { return new OpenSearchStorageEngine(client(), settings); } @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public ExecutionEngine executionEngine() { OpenSearchFunctions.register(functionRepository); return new OpenSearchExecutionEngine(client(), protector()); } @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public ResourceMonitor resourceMonitor() { return new OpenSearchResourceMonitor(settings, new OpenSearchMemoryHealthy()); } @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public ExecutionProtector protector() { return new OpenSearchExecutionProtector(resourceMonitor()); } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/OpenSearchPluginConfig.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/OpenSearchPluginConfig.java deleted file mode 100644 index 24d7e4e7f5..0000000000 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/OpenSearchPluginConfig.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - - -package org.opensearch.sql.plugin.rest; - -import org.opensearch.client.node.NodeClient; -import org.opensearch.sql.common.setting.Settings; -import org.opensearch.sql.executor.ExecutionEngine; -import org.opensearch.sql.monitor.ResourceMonitor; -import org.opensearch.sql.opensearch.client.OpenSearchClient; -import org.opensearch.sql.opensearch.client.OpenSearchNodeClient; -import org.opensearch.sql.opensearch.executor.OpenSearchExecutionEngine; -import org.opensearch.sql.opensearch.executor.protector.ExecutionProtector; -import org.opensearch.sql.opensearch.executor.protector.OpenSearchExecutionProtector; -import org.opensearch.sql.opensearch.monitor.OpenSearchMemoryHealthy; -import org.opensearch.sql.opensearch.monitor.OpenSearchResourceMonitor; -import org.opensearch.sql.opensearch.storage.OpenSearchStorageEngine; -import org.opensearch.sql.storage.StorageEngine; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -/** - * OpenSearch plugin config that injects cluster service and node client from plugin - * and initialize OpenSearch storage and execution engine. - */ -@Configuration -public class OpenSearchPluginConfig { - - @Autowired - private NodeClient nodeClient; - - @Autowired - private Settings settings; - - @Bean - public OpenSearchClient client() { - return new OpenSearchNodeClient(nodeClient); - } - - @Bean - public ExecutionEngine executionEngine() { - return new OpenSearchExecutionEngine(client(), protector()); - } - - @Bean - public ResourceMonitor resourceMonitor() { - return new OpenSearchResourceMonitor(settings, new OpenSearchMemoryHealthy()); - } - - @Bean - public ExecutionProtector protector() { - return new OpenSearchExecutionProtector(resourceMonitor()); - } -} diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java index e9202d96e8..2bc3c8d72d 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java @@ -39,7 +39,6 @@ import org.opensearch.sql.plugin.transport.PPLQueryAction; import org.opensearch.sql.plugin.transport.TransportPPLQueryRequest; import org.opensearch.sql.plugin.transport.TransportPPLQueryResponse; -import org.opensearch.sql.ppl.domain.PPLQueryRequest; public class RestPPLQueryAction extends BaseRestHandler { public static final String QUERY_API_ENDPOINT = "/_plugins/_ppl"; diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index eaad009216..af57c91e5c 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -7,8 +7,6 @@ import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; -import java.io.IOException; -import java.security.PrivilegedExceptionAction; import java.util.Locale; import java.util.Optional; import org.opensearch.action.ActionListener; @@ -18,7 +16,6 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; -import org.opensearch.sql.catalog.CatalogService; import org.opensearch.sql.common.response.ResponseListener; import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.common.utils.QueryContext; @@ -27,10 +24,7 @@ import org.opensearch.sql.legacy.metrics.Metrics; import org.opensearch.sql.opensearch.security.SecurityAccess; import org.opensearch.sql.opensearch.setting.OpenSearchSettings; -import org.opensearch.sql.plugin.catalog.CatalogServiceImpl; -import org.opensearch.sql.plugin.rest.OpenSearchPluginConfig; import org.opensearch.sql.ppl.PPLService; -import org.opensearch.sql.ppl.config.PPLServiceConfig; import org.opensearch.sql.ppl.domain.PPLQueryRequest; import org.opensearch.sql.protocol.response.QueryResult; import org.opensearch.sql.protocol.response.format.CsvResponseFormatter; @@ -55,6 +49,8 @@ public class TransportPPLQueryAction /** Settings required by been initialization. */ private final Settings pluginSettings; + private final AnnotationConfigApplicationContext applicationContext; + /** Constructor of TransportPPLQueryAction. */ @Inject @@ -63,11 +59,12 @@ public TransportPPLQueryAction( ActionFilters actionFilters, NodeClient client, ClusterService clusterService, - org.opensearch.common.settings.Settings clusterSettings) { + AnnotationConfigApplicationContext applicationContext) { super(PPLQueryAction.NAME, transportService, actionFilters, TransportPPLQueryRequest::new); this.client = client; this.clusterService = clusterService; this.pluginSettings = new OpenSearchSettings(clusterService.getClusterSettings()); + this.applicationContext = applicationContext; } /** @@ -82,7 +79,8 @@ protected void doExecute( QueryContext.addRequestId(); - PPLService pplService = createPPLService(client); + PPLService pplService = + SecurityAccess.doPrivileged(() -> applicationContext.getBean(PPLService.class)); TransportPPLQueryRequest transportRequest = TransportPPLQueryRequest.fromActionRequest(request); // in order to use PPL service, we need to convert TransportPPLQueryRequest to PPLQueryRequest PPLQueryRequest transformedRequest = transportRequest.toPPLQueryRequest(); @@ -94,29 +92,6 @@ protected void doExecute( } } - private PPLService createPPLService(NodeClient client) { - return doPrivileged( - () -> { - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); - context.registerBean(ClusterService.class, () -> clusterService); - context.registerBean(NodeClient.class, () -> client); - context.registerBean(Settings.class, () -> pluginSettings); - context.registerBean(CatalogService.class, CatalogServiceImpl::getInstance); - context.register(OpenSearchPluginConfig.class); - context.register(PPLServiceConfig.class); - context.refresh(); - return context.getBean(PPLService.class); - }); - } - - private T doPrivileged(PrivilegedExceptionAction action) { - try { - return SecurityAccess.doPrivileged(action); - } catch (IOException e) { - throw new IllegalStateException("Failed to perform privileged action", e); - } - } - /** * TODO: need to extract an interface for both SQL and PPL action handler and move these common * methods to the interface. This is not easy to do now because SQL action handler is still in diff --git a/ppl/build.gradle b/ppl/build.gradle index 12b0787efc..2c3c648478 100644 --- a/ppl/build.gradle +++ b/ppl/build.gradle @@ -46,7 +46,6 @@ dependencies { implementation "org.antlr:antlr4-runtime:4.7.1" implementation group: 'com.google.guava', name: 'guava', version: '31.0.1-jre' - api group: 'org.opensearch', name: 'opensearch-x-content', version: "${opensearch_version}" api group: 'org.json', name: 'json', version: '20180813' implementation group: 'org.springframework', name: 'spring-context', version: "${spring_version}" implementation group: 'org.springframework', name: 'spring-beans', version: "${spring_version}" diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/config/PPLServiceConfig.java b/ppl/src/main/java/org/opensearch/sql/ppl/config/PPLServiceConfig.java index bd6c4e3937..1c01ccce89 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/config/PPLServiceConfig.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/config/PPLServiceConfig.java @@ -13,9 +13,11 @@ import org.opensearch.sql.ppl.PPLService; import org.opensearch.sql.ppl.antlr.PPLSyntaxParser; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Scope; @Configuration @Import({ExpressionConfig.class}) @@ -37,6 +39,7 @@ public class PPLServiceConfig { * @return PPLService. */ @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public PPLService pplService() { return new PPLService(new PPLSyntaxParser(), executionEngine, functionRepository, catalogService); diff --git a/sql/src/main/java/org/opensearch/sql/sql/config/SQLServiceConfig.java b/sql/src/main/java/org/opensearch/sql/sql/config/SQLServiceConfig.java index 2d22d92081..f7845701d2 100644 --- a/sql/src/main/java/org/opensearch/sql/sql/config/SQLServiceConfig.java +++ b/sql/src/main/java/org/opensearch/sql/sql/config/SQLServiceConfig.java @@ -14,11 +14,12 @@ import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.sql.SQLService; import org.opensearch.sql.sql.antlr.SQLSyntaxParser; -import org.opensearch.sql.storage.StorageEngine; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Scope; /** * SQL service configuration for Spring container initialization. @@ -36,11 +37,6 @@ public class SQLServiceConfig { @Autowired private BuiltinFunctionRepository functionRepository; - @Bean - public Analyzer analyzer() { - return new Analyzer(new ExpressionAnalyzer(functionRepository), catalogService); - } - /** * The registration of OpenSearch storage engine happens here because * OpenSearchStorageEngine is dependent on NodeClient. @@ -48,8 +44,12 @@ public Analyzer analyzer() { * @return SQLService. */ @Bean + @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) public SQLService sqlService() { - return new SQLService(new SQLSyntaxParser(), analyzer(), executionEngine, + return new SQLService( + new SQLSyntaxParser(), + new Analyzer(new ExpressionAnalyzer(functionRepository), catalogService), + executionEngine, functionRepository); }