Skip to content

Commit 38d39d4

Browse files
committed
Make analytics-engine an optional dependency via AnalyticsFrontEndExtension SPI
End-to-end consumer-side wiring for the AnalyticsFrontEndExtension SPI (analytics-framework). Lets the SQL plugin install and run on stock OpenSearch distributions that don't ship analytics-engine, while preserving the analytics routing behavior when both plugins are co-installed. Build wiring (plugin/build.gradle): - Mark analytics-engine as ;optional=true in extendedPlugins. OpenSearch's PluginsService.checkBundleJarHell skips both URL-overlap and class-level JarHell against optional deps, so SQL bundles its own copies of the previously-stripped jars (Calcite, Guava, etc.) again. - Drop the entire bundlePlugin exclusion list — the hand-maintained jar-deduplication hack goes away with the SPI. - Vendor the rebuilt analytics-framework-3.7.0-SNAPSHOT.jar (with the new AnalyticsFrontEndExtension + AnalyticsServices types) and the rebuilt analytics-engine-3.7.0-SNAPSHOT.zip (with the producer-side Guice TypeListener that pushes services to consumers). SPI consumer (SQLAnalyticsFrontEndExtension.java + META-INF/services registration): - Implements AnalyticsFrontEndExtension; setAnalyticsServices stashes the executor + schemaProvider into AnalyticsExecutorHolder. - Kept in a class separate from SQLPlugin so SQLPlugin's bytecode doesn't reference any analytics-framework class. When AE is absent, ServiceLoader never touches this class and SQL boots without analytics-framework on its runtime classpath. Holder isolation (AnalyticsExecutorHolder.java): - Refactored to store services as Object internally — no analytics-framework type appears in any signature loaded at SQL plugin startup. Callers cast at use sites that are already gated on a non-null value. - Added schemaProvider alongside the executor. Lazy resolution at the request boundary: - TransportPPLQueryAction: drop the @Inject QueryPlanExecutor constructor parameter (the cross-plugin Guice dependency that broke the optional path). Replace with analyticsHandler() — null-checked lazy resolution from the holder; falls through to the legacy PPL path when AE is absent. - SQLPlugin#createSqlAnalyticsRouter: same lazy-supplier pattern against the new Object-typed holder + RestUnifiedQueryAction factory. - RestUnifiedQueryAction: add fromUnknownExecutor(Object, Object) factory (the only cast site for the analytics-framework types). Replaced static OpenSearchSchemaBuilder.buildSchema() call with the injected SchemaProvider so the runtime no longer has a hard static reference to analytics-engine. Verified end-to-end with bin/opensearch-plugin install on a real OpenSearch 3.7 distribution: - WITH analytics-engine: all three plugins install; Lucene PPL works; parquet_* PPL routes through the analytics engine (confirmed via /_plugins/_ppl/_explain showing Calcite logical plan). - WITHOUT analytics-engine: SQL + job-scheduler install (warning logged for missing optional AE); Lucene PPL works; parquet_* PPL falls through to legacy with a clean IndexNotFoundException — no NoClassDefFoundError, no startup crash. Pairs with the producer-side wiring in opensearch-project/OpenSearch. Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent e113b7f commit 38d39d4

10 files changed

Lines changed: 159 additions & 46 deletions

File tree

1.21 KB
Binary file not shown.
2.19 KB
Binary file not shown.

plugin/build.gradle

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,21 @@ opensearchplugin {
5555
name 'opensearch-sql'
5656
description 'OpenSearch SQL'
5757
classname 'org.opensearch.sql.plugin.SQLPlugin'
58-
extendedPlugins = ['opensearch-job-scheduler', 'analytics-engine']
58+
extendedPlugins = ['opensearch-job-scheduler', 'analytics-engine;optional=true']
5959
licenseFile rootProject.file("LICENSE.txt")
6060
noticeFile rootProject.file("NOTICE")
6161
}
6262

63-
// Exclude jars provided by the analytics-engine plugin (shared via extendedPlugins classloader).
64-
// MUST match what's actually in the analytics-engine ZIP to avoid both jar hell (when analytics-engine
65-
// ships it) and ClassNotFoundException at runtime (when it doesn't). Last verified against
66-
// analytics-engine-3.7.0-SNAPSHOT — keep this list aligned when bumping versions.
63+
// Strip jars provided by analytics-engine to avoid runtime classloader confusion when both
64+
// plugins are co-installed (Janino's runtime code generation breaks when two versions of
65+
// Calcite/Janino are visible). With extendedPlugins ';optional=true', JarHell skips overlap
66+
// checks but Janino still ends up confused, so we keep this hand-maintained exclusion list as
67+
// the lesser evil. When analytics-engine is absent, classes that need these jars remain unloaded
68+
// because the SPI consumer (SQLAnalyticsFrontEndExtension) never fires and analytics-routing code
69+
// paths stay inert via the holder's null gate.
70+
//
71+
// When bumping analytics-engine, re-verify this list against `unzip -l <new analytics-engine zip>`
72+
// — anything analytics-engine ships at runtime must appear here to prevent dual-version conflicts.
6773
bundlePlugin {
6874
exclude 'calcite-core-*.jar'
6975
exclude 'calcite-linq4j-*.jar'
@@ -81,15 +87,10 @@ bundlePlugin {
8187
exclude 'jackson-annotations-*.jar'
8288
exclude 'jackson-databind-*.jar'
8389
exclude 'httpcore5-5*.jar'
84-
// TODO: Remove the three httpcore5/httpclient5 exclusions below — and ideally this entire
85-
// bundlePlugin exclusion block — once analytics-engine becomes an optional dependency via the
86-
// AnalyticsFrontEndExtension SPI (opensearch-project/OpenSearch#21449). The shared-classloader
87-
// jar deduplication that requires this hand-maintained list goes away with the SPI.
8890
exclude 'httpcore5-h2-*.jar'
8991
exclude 'httpcore5-reactive-*.jar'
9092
exclude 'httpclient5-*.jar'
9193
exclude 'commons-text-*.jar'
92-
// Calcite transitive deps now bundled in analytics-engine 3.7 — exclude from sql to avoid jar hell.
9394
exclude 'commons-math3-*.jar'
9495
exclude 'commons-dbcp2-*.jar'
9596
exclude 'commons-pool2-*.jar'
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.plugin;
7+
8+
import org.opensearch.analytics.spi.AnalyticsFrontEndExtension;
9+
import org.opensearch.analytics.spi.AnalyticsServices;
10+
import org.opensearch.sql.plugin.rest.AnalyticsExecutorHolder;
11+
12+
/**
13+
* SPI consumer that publishes the analytics-engine services into {@link AnalyticsExecutorHolder}
14+
* when analytics-engine is installed.
15+
*
16+
* <p>Kept separate from {@link SQLPlugin} so that SQLPlugin's bytecode does not reference any
17+
* analytics-framework class. When analytics-engine is absent, OpenSearch never invokes {@code
18+
* ServiceLoader.load(AnalyticsFrontEndExtension.class)} (because no plugin defining the SPI is
19+
* present), so this class is never resolved and SQL boots without needing analytics-framework on
20+
* its runtime classpath.
21+
*/
22+
public class SQLAnalyticsFrontEndExtension implements AnalyticsFrontEndExtension {
23+
24+
@Override
25+
public void setAnalyticsServices(AnalyticsServices services) {
26+
AnalyticsExecutorHolder.set(services.queryPlanExecutor(), services.schemaProvider());
27+
}
28+
}

plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,14 @@ private BiFunction<SQLQueryRequest, RestChannel, Boolean> createSqlAnalyticsRout
204204
java.util.function.Supplier<RestUnifiedQueryAction> handlerSupplier =
205205
() -> {
206206
if (cached[0] == null) {
207-
var executor = AnalyticsExecutorHolder.get();
208-
if (executor == null) {
207+
Object executor = AnalyticsExecutorHolder.getQueryPlanExecutor();
208+
Object schemaProvider = AnalyticsExecutorHolder.getSchemaProvider();
209+
if (executor == null || schemaProvider == null) {
209210
return null;
210211
}
211-
cached[0] = new RestUnifiedQueryAction(client, clusterService, executor);
212+
cached[0] =
213+
RestUnifiedQueryAction.fromUnknownExecutor(
214+
client, clusterService, executor, schemaProvider);
212215
}
213216
return cached[0];
214217
};

plugin/src/main/java/org/opensearch/sql/plugin/rest/AnalyticsExecutorHolder.java

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,32 +5,48 @@
55

66
package org.opensearch.sql.plugin.rest;
77

8-
import org.apache.calcite.rel.RelNode;
9-
import org.opensearch.analytics.exec.QueryPlanExecutor;
10-
118
/**
12-
* Bridge for sharing the analytics-engine {@link QueryPlanExecutor} between the PPL transport
13-
* action (where Guice resolves the binding via {@code @Inject}) and the REST-only SQL router (where
14-
* Guice cannot, because {@code SQLPlugin#getRestHandlers} runs before the Node-level injector
15-
* satisfies {@code @Inject} parameters).
9+
* Static bridge that lets {@code SQLAnalyticsFrontEndExtension} (the SPI consumer) hand off the
10+
* analytics-engine services to the SQL request paths that need them.
1611
*
17-
* <p>Why a static holder: cross-plugin Guice injection needs a class registered in the Node
18-
* injector, and {@link org.opensearch.sql.plugin.SQLPlugin}'s SQL routing path is built in {@code
19-
* getRestHandlers} — outside any Guice-managed lifecycle. Persisting the executor in this holder
20-
* once {@link org.opensearch.sql.plugin.transport.TransportPPLQueryAction} is constructed lets the
21-
* SQL router read the same instance without going back through the injector.
12+
* <p>Stored as {@link Object} on purpose. Callers cast at use sites that are already gated on a
13+
* non-null value, ensuring no analytics-framework class is referenced from any signature loaded at
14+
* SQL plugin startup. When analytics-engine is not installed, the SPI never fires, the holder stays
15+
* null, and {@code TransportPPLQueryAction} / {@code SQLPlugin#createSqlAnalyticsRouter} fall
16+
* through to the legacy paths without ever touching analytics-framework types.
2217
*/
2318
public final class AnalyticsExecutorHolder {
2419

25-
private static volatile QueryPlanExecutor<RelNode, Iterable<Object[]>> executor;
20+
private static volatile Object queryPlanExecutor;
21+
private static volatile Object schemaProvider;
2622

2723
private AnalyticsExecutorHolder() {}
2824

29-
public static void set(QueryPlanExecutor<RelNode, Iterable<Object[]>> instance) {
30-
executor = instance;
25+
/**
26+
* Set both services in one call. Invoked by {@code SQLAnalyticsFrontEndExtension} from the SPI
27+
* push lifecycle. Either argument may be {@code null} (not expected today, but tolerated).
28+
*/
29+
public static void set(Object queryPlanExecutor, Object schemaProvider) {
30+
AnalyticsExecutorHolder.queryPlanExecutor = queryPlanExecutor;
31+
AnalyticsExecutorHolder.schemaProvider = schemaProvider;
32+
}
33+
34+
/**
35+
* Returns the analytics-engine query plan executor as {@link Object}. Returns {@code null} when
36+
* analytics-engine is not installed (SPI never fired). Callers cast to {@code
37+
* QueryPlanExecutor<RelNode, Iterable<Object[]>>} only inside code paths that are gated on a
38+
* non-null return value — see {@code RestUnifiedQueryAction#fromUnknownExecutor}.
39+
*/
40+
public static Object getQueryPlanExecutor() {
41+
return queryPlanExecutor;
3142
}
3243

33-
public static QueryPlanExecutor<RelNode, Iterable<Object[]>> get() {
34-
return executor;
44+
/**
45+
* Returns the analytics-engine schema provider as {@link Object}. Returns {@code null} when
46+
* analytics-engine is not installed. Callers cast to {@code SchemaProvider} only inside code
47+
* paths that are gated on a non-null return value.
48+
*/
49+
public static Object getSchemaProvider() {
50+
return schemaProvider;
3551
}
3652
}

plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.apache.logging.log4j.Logger;
2424
import org.apache.logging.log4j.ThreadContext;
2525
import org.opensearch.analytics.exec.QueryPlanExecutor;
26-
import org.opensearch.analytics.schema.OpenSearchSchemaBuilder;
26+
import org.opensearch.analytics.schema.SchemaProvider;
2727
import org.opensearch.cluster.service.ClusterService;
2828
import org.opensearch.common.unit.TimeValue;
2929
import org.opensearch.core.action.ActionListener;
@@ -59,14 +59,33 @@ public class RestUnifiedQueryAction {
5959
private final AnalyticsExecutionEngine analyticsEngine;
6060
private final NodeClient client;
6161
private final ClusterService clusterService;
62+
private final SchemaProvider schemaProvider;
6263

6364
public RestUnifiedQueryAction(
6465
NodeClient client,
6566
ClusterService clusterService,
66-
QueryPlanExecutor<RelNode, Iterable<Object[]>> planExecutor) {
67+
QueryPlanExecutor<RelNode, Iterable<Object[]>> planExecutor,
68+
SchemaProvider schemaProvider) {
6769
this.client = client;
6870
this.clusterService = clusterService;
6971
this.analyticsEngine = new AnalyticsExecutionEngine(planExecutor);
72+
this.schemaProvider = schemaProvider;
73+
}
74+
75+
/**
76+
* Construct from holder-stashed services typed as {@link Object} so callers don't take a
77+
* compile-time reference on analytics-framework. The cast is confined to this method — invoking
78+
* it loads the analytics-framework types for the first time, and the caller must gate on a
79+
* non-null executor (i.e. analytics-engine is installed).
80+
*/
81+
@SuppressWarnings("unchecked")
82+
public static RestUnifiedQueryAction fromUnknownExecutor(
83+
NodeClient client, ClusterService clusterService, Object executor, Object schemaProvider) {
84+
return new RestUnifiedQueryAction(
85+
client,
86+
clusterService,
87+
(QueryPlanExecutor<RelNode, Iterable<Object[]>>) executor,
88+
(SchemaProvider) schemaProvider);
7089
}
7190

7291
/**
@@ -161,7 +180,7 @@ private static UnifiedQueryContext buildParsingContext(QueryType queryType) {
161180
private UnifiedQueryContext buildContext(QueryType queryType, boolean profiling) {
162181
return UnifiedQueryContext.builder()
163182
.language(queryType)
164-
.catalog(SCHEMA_NAME, OpenSearchSchemaBuilder.buildSchema(clusterService.state()))
183+
.catalog(SCHEMA_NAME, schemaProvider.buildSchema(clusterService.state()))
165184
.defaultNamespace(SCHEMA_NAME)
166185
.profiling(profiling)
167186
.build();

plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,9 @@
1313
import java.util.Locale;
1414
import java.util.Optional;
1515
import java.util.function.Supplier;
16-
import org.apache.calcite.rel.RelNode;
1716
import org.opensearch.action.ActionRequest;
1817
import org.opensearch.action.support.ActionFilters;
1918
import org.opensearch.action.support.HandledTransportAction;
20-
import org.opensearch.analytics.exec.QueryPlanExecutor;
2119
import org.opensearch.cluster.service.ClusterService;
2220
import org.opensearch.common.inject.Guice;
2321
import org.opensearch.common.inject.Inject;
@@ -62,7 +60,16 @@ public class TransportPPLQueryAction
6260

6361
private final Supplier<Boolean> pplEnabled;
6462

65-
private final RestUnifiedQueryAction unifiedQueryHandler;
63+
private final NodeClient client;
64+
65+
private final ClusterService clusterService;
66+
67+
/**
68+
* Lazily-resolved analytics handler. {@code null} until the first analytics-routed request, and
69+
* remains {@code null} forever if analytics-engine is not installed (the SPI never fires, so
70+
* {@link AnalyticsExecutorHolder#getQueryPlanExecutor()} stays null).
71+
*/
72+
private volatile RestUnifiedQueryAction unifiedQueryHandler;
6673

6774
/** Constructor of TransportPPLQueryAction. */
6875
@Inject
@@ -72,10 +79,12 @@ public TransportPPLQueryAction(
7279
NodeClient client,
7380
ClusterService clusterService,
7481
DataSourceServiceImpl dataSourceService,
75-
org.opensearch.common.settings.Settings clusterSettings,
76-
QueryPlanExecutor<RelNode, Iterable<Object[]>> queryPlanExecutor) {
82+
org.opensearch.common.settings.Settings clusterSettings) {
7783
super(PPLQueryAction.NAME, transportService, actionFilters, TransportPPLQueryRequest::new);
7884

85+
this.client = client;
86+
this.clusterService = clusterService;
87+
7988
ModulesBuilder modules = new ModulesBuilder();
8089
modules.add(new OpenSearchPluginModule());
8190
modules.add(
@@ -86,9 +95,6 @@ public TransportPPLQueryAction(
8695
b.bind(DataSourceService.class).toInstance(dataSourceService);
8796
});
8897
this.injector = Guice.createInjector(modules);
89-
AnalyticsExecutorHolder.set(queryPlanExecutor);
90-
this.unifiedQueryHandler =
91-
new RestUnifiedQueryAction(client, clusterService, queryPlanExecutor);
9298
this.pplEnabled =
9399
() ->
94100
MULTI_ALLOW_EXPLICIT_INDEX.get(clusterSettings)
@@ -98,6 +104,31 @@ public TransportPPLQueryAction(
98104
.getSettingValue(Settings.Key.PPL_ENABLED);
99105
}
100106

107+
/**
108+
* Resolves the analytics-engine-backed handler lazily. Returns {@code null} when analytics-engine
109+
* is not installed; callers fall through to the legacy PPL pipeline. Synchronized double-checked
110+
* cache so we only build the handler once on the first analytics request.
111+
*/
112+
private RestUnifiedQueryAction analyticsHandler() {
113+
RestUnifiedQueryAction cached = unifiedQueryHandler;
114+
if (cached != null) {
115+
return cached;
116+
}
117+
Object executor = AnalyticsExecutorHolder.getQueryPlanExecutor();
118+
Object schemaProvider = AnalyticsExecutorHolder.getSchemaProvider();
119+
if (executor == null || schemaProvider == null) {
120+
return null;
121+
}
122+
synchronized (this) {
123+
if (unifiedQueryHandler == null) {
124+
unifiedQueryHandler =
125+
RestUnifiedQueryAction.fromUnknownExecutor(
126+
client, clusterService, executor, schemaProvider);
127+
}
128+
return unifiedQueryHandler;
129+
}
130+
}
131+
101132
/**
102133
* {@inheritDoc} Transform the request and call super.doExecute() to support call from other
103134
* plugins.
@@ -134,16 +165,20 @@ protected void doExecute(
134165
QueryContext.setProfile(transformedRequest.profile());
135166
ActionListener<TransportPPLQueryResponse> clearingListener = wrapWithProfilingClear(listener);
136167

137-
// Route to analytics engine for non-Lucene (e.g., Parquet-backed) indices
138-
if (unifiedQueryHandler.isAnalyticsIndex(transformedRequest.getRequest(), QueryType.PPL)) {
168+
// Route to analytics engine for non-Lucene (e.g., Parquet-backed) indices.
169+
// analyticsHandler() returns null when analytics-engine isn't installed — we fall through
170+
// to the regular Lucene PPL path so non-analytics queries still work.
171+
RestUnifiedQueryAction analytics = analyticsHandler();
172+
if (analytics != null
173+
&& analytics.isAnalyticsIndex(transformedRequest.getRequest(), QueryType.PPL)) {
139174
if (transformedRequest.isExplainRequest()) {
140-
unifiedQueryHandler.explain(
175+
analytics.explain(
141176
transformedRequest.getRequest(),
142177
QueryType.PPL,
143178
transformedRequest.mode(),
144179
createExplainResponseListener(transformedRequest, clearingListener));
145180
} else {
146-
unifiedQueryHandler.execute(
181+
analytics.execute(
147182
transformedRequest.getRequest(),
148183
QueryType.PPL,
149184
transformedRequest.profile(),
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#
2+
# Copyright OpenSearch Contributors
3+
# SPDX-License-Identifier: Apache-2.0
4+
#
5+
6+
org.opensearch.sql.plugin.SQLAnalyticsFrontEndExtension

plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.junit.Before;
1414
import org.junit.Test;
1515
import org.opensearch.analytics.exec.QueryPlanExecutor;
16+
import org.opensearch.analytics.schema.SchemaProvider;
1617
import org.opensearch.cluster.service.ClusterService;
1718
import org.opensearch.sql.executor.QueryType;
1819
import org.opensearch.transport.client.node.NodeClient;
@@ -30,7 +31,11 @@ public void setUp() {
3031
@SuppressWarnings("unchecked")
3132
QueryPlanExecutor<RelNode, Iterable<Object[]>> executor = mock(QueryPlanExecutor.class);
3233
action =
33-
new RestUnifiedQueryAction(mock(NodeClient.class), mock(ClusterService.class), executor);
34+
new RestUnifiedQueryAction(
35+
mock(NodeClient.class),
36+
mock(ClusterService.class),
37+
executor,
38+
mock(SchemaProvider.class));
3439
}
3540

3641
@Test

0 commit comments

Comments
 (0)