Skip to content

Commit 280d3b4

Browse files
committed
Make analytics-engine an optional dependency
sql plugin previously declared analytics-engine as a hard dependency: extendedPlugins = ['opensearch-job-scheduler', 'analytics-engine'] Installing opensearch-sql on a distribution that doesn't ship analytics-engine failed with: Missing plugin [analytics-engine], dependency of [opensearch-sql] Marking the dep ;optional=true alone is not enough — TransportPPLQueryAction Guice-injects QueryPlanExecutor on its constructor, and Guice's OpenSearch fork rejects a required constructor parameter whose binding is missing at injector-build time ("constructors cannot be optional"). Move QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter. Guice invokes the setter only when a binding for QueryPlanExecutor<RelNode, Iterable<Object[]>> exists — i.e. when analytics-engine's createGuiceModules has run and bound DefaultPlanExecutor. Absent analytics-engine, the setter is silently skipped, unifiedQueryHandler stays null, and all PPL queries route to the v2 Calcite-to-OpenSearch path already in the sql plugin. Drop the bundlePlugin exclude list. OpenSearch's jar-hell check skips the extended-plugin cross-check when the dep is marked optional (PluginsService.java:763), so sql can bundle every jar it needs to run self-sufficiently. When analytics-engine is installed, parent-first classloader delegation still lets analytics-engine's copies win for any shared class; sql's bundled copies sit idle. Promote analytics-framework.jar from compileOnly to api so QueryPlanExecutor is reachable from sql's own classloader when the plugin is absent. analytics-engine.jar stays compileOnly (required only for OpenSearchSchemaBuilder, which lives in the engine plugin and is reached only through RestUnifiedQueryAction — a class that never loads when the setter is skipped). Validated on a live 2-node cluster in both configurations: - With analytics-engine installed: legacy and analytics PPL both return expected rows; routing to the analytics path still fires for parquet_-prefixed indices. - Without analytics-engine (only opensearch-job-scheduler + opensearch-sql installed): cluster starts cleanly, PPL and SQL queries against Lucene indices return expected rows, parquet_-prefixed lookups return a clean IndexNotFoundException instead of a NullPointerException or NoClassDefFoundError. Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
1 parent e113b7f commit 280d3b4

2 files changed

Lines changed: 50 additions & 53 deletions

File tree

plugin/build.gradle

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -55,53 +55,16 @@ 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.
67-
bundlePlugin {
68-
exclude 'calcite-core-*.jar'
69-
exclude 'calcite-linq4j-*.jar'
70-
exclude 'avatica-core-*.jar'
71-
exclude 'guava-*.jar'
72-
exclude 'failureaccess-*.jar'
73-
exclude 'slf4j-api-*.jar'
74-
exclude 'commons-codec-*.jar'
75-
exclude 'commons-compiler-*.jar'
76-
exclude 'commons-io-*.jar'
77-
exclude 'commons-lang3-*.jar'
78-
exclude 'janino-*.jar'
79-
exclude 'joou-java-6-*.jar'
80-
exclude 'json-path-*.jar'
81-
exclude 'jackson-annotations-*.jar'
82-
exclude 'jackson-databind-*.jar'
83-
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.
88-
exclude 'httpcore5-h2-*.jar'
89-
exclude 'httpcore5-reactive-*.jar'
90-
exclude 'httpclient5-*.jar'
91-
exclude 'commons-text-*.jar'
92-
// Calcite transitive deps now bundled in analytics-engine 3.7 — exclude from sql to avoid jar hell.
93-
exclude 'commons-math3-*.jar'
94-
exclude 'commons-dbcp2-*.jar'
95-
exclude 'commons-pool2-*.jar'
96-
exclude 'uzaygezen-core-*.jar'
97-
exclude 'sketches-core-*.jar'
98-
exclude 'memory-0*.jar'
99-
exclude 'jts-io-common-*.jar'
100-
exclude 'proj4j-*.jar'
101-
exclude 'json-smart-*.jar'
102-
exclude 'accessors-smart-*.jar'
103-
exclude 'asm-9*.jar'
104-
}
63+
// With analytics-engine declared ;optional=true, OpenSearch's jar-hell check skips the
64+
// extended-plugin cross-check (PluginsService.java:763). sql can safely bundle every jar it
65+
// needs; when analytics-engine IS present, parent-first classloader delegation means
66+
// analytics-engine's copy of any shared class wins and sql's bundled copy sits idle.
67+
// When analytics-engine is ABSENT, sql's own copies provide the classes sql needs to run.
10568

10669
publishing {
10770
publications {
@@ -204,7 +167,14 @@ dependencies {
204167

205168
api project(":ppl")
206169
api project(':api')
207-
compileOnly files("${rootDir}/libs/analytics-framework-3.7.0-SNAPSHOT.jar")
170+
// analytics-framework: interfaces (QueryPlanExecutor, SchemaProvider) that sql imports.
171+
// Bundled so sql still compiles + installs when analytics-engine is absent (;optional=true).
172+
// When analytics-engine IS present, its copy wins via parent-first classloader delegation,
173+
// so there's no class-identity conflict at runtime.
174+
api files("${rootDir}/libs/analytics-framework-3.7.0-SNAPSHOT.jar")
175+
// analytics-engine: only needed at compile time for OpenSearchSchemaBuilder, which lives in
176+
// the engine plugin (not the framework lib). Stays compileOnly — must NOT be bundled because
177+
// its loading depends on the plugin being installed.
208178
compileOnly files("${rootDir}/libs/analytics-engine-3.7.0-SNAPSHOT.jar")
209179
api project(':legacy')
210180
api project(':opensearch')

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

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,32 @@ public class TransportPPLQueryAction
6262

6363
private final Supplier<Boolean> pplEnabled;
6464

65-
private final RestUnifiedQueryAction unifiedQueryHandler;
65+
// Populated by {@link #setQueryPlanExecutor} (Guice optional method injection) after
66+
// constructor runs. Remains null when analytics-engine plugin is absent — Guice skips
67+
// optional setters whose bindings don't exist, so the plugin loads cleanly without analytics.
68+
private volatile RestUnifiedQueryAction unifiedQueryHandler;
69+
private final NodeClient clientRef;
70+
private final ClusterService clusterServiceRef;
6671

67-
/** Constructor of TransportPPLQueryAction. */
72+
/**
73+
* Constructor of TransportPPLQueryAction.
74+
*
75+
* <p>Does NOT {@code @Inject QueryPlanExecutor} on the constructor — Guice rejects missing
76+
* bindings at injector-build time, so a required constructor dep would fail when
77+
* analytics-engine plugin is absent. Instead we use optional method injection on
78+
* {@link #setQueryPlanExecutor}: Guice calls it only if the binding exists.
79+
*/
6880
@Inject
6981
public TransportPPLQueryAction(
7082
TransportService transportService,
7183
ActionFilters actionFilters,
7284
NodeClient client,
7385
ClusterService clusterService,
7486
DataSourceServiceImpl dataSourceService,
75-
org.opensearch.common.settings.Settings clusterSettings,
76-
QueryPlanExecutor<RelNode, Iterable<Object[]>> queryPlanExecutor) {
87+
org.opensearch.common.settings.Settings clusterSettings) {
7788
super(PPLQueryAction.NAME, transportService, actionFilters, TransportPPLQueryRequest::new);
89+
this.clientRef = client;
90+
this.clusterServiceRef = clusterService;
7891

7992
ModulesBuilder modules = new ModulesBuilder();
8093
modules.add(new OpenSearchPluginModule());
@@ -86,9 +99,6 @@ public TransportPPLQueryAction(
8699
b.bind(DataSourceService.class).toInstance(dataSourceService);
87100
});
88101
this.injector = Guice.createInjector(modules);
89-
AnalyticsExecutorHolder.set(queryPlanExecutor);
90-
this.unifiedQueryHandler =
91-
new RestUnifiedQueryAction(client, clusterService, queryPlanExecutor);
92102
this.pplEnabled =
93103
() ->
94104
MULTI_ALLOW_EXPLICIT_INDEX.get(clusterSettings)
@@ -98,6 +108,21 @@ public TransportPPLQueryAction(
98108
.getSettingValue(Settings.Key.PPL_ENABLED);
99109
}
100110

111+
/**
112+
* Optional method injection target. Guice invokes this ONLY when a binding for {@code
113+
* QueryPlanExecutor<RelNode, Iterable<Object[]>>} exists — i.e. when analytics-engine plugin
114+
* is installed and its {@code createGuiceModules} has bound {@code DefaultPlanExecutor}.
115+
* Absent analytics-engine, Guice silently skips the call; {@link #unifiedQueryHandler}
116+
* stays null and all PPL routing falls through to the legacy engine.
117+
*/
118+
@Inject(optional = true)
119+
public void setQueryPlanExecutor(
120+
QueryPlanExecutor<RelNode, Iterable<Object[]>> queryPlanExecutor) {
121+
AnalyticsExecutorHolder.set(queryPlanExecutor);
122+
this.unifiedQueryHandler =
123+
new RestUnifiedQueryAction(clientRef, clusterServiceRef, queryPlanExecutor);
124+
}
125+
101126
/**
102127
* {@inheritDoc} Transform the request and call super.doExecute() to support call from other
103128
* plugins.
@@ -134,8 +159,10 @@ protected void doExecute(
134159
QueryContext.setProfile(transformedRequest.profile());
135160
ActionListener<TransportPPLQueryResponse> clearingListener = wrapWithProfilingClear(listener);
136161

137-
// Route to analytics engine for non-Lucene (e.g., Parquet-backed) indices
138-
if (unifiedQueryHandler.isAnalyticsIndex(transformedRequest.getRequest(), QueryType.PPL)) {
162+
// Route to analytics engine for non-Lucene (e.g., Parquet-backed) indices.
163+
// Skipped entirely when analytics-engine plugin is absent — unifiedQueryHandler is null.
164+
if (unifiedQueryHandler != null
165+
&& unifiedQueryHandler.isAnalyticsIndex(transformedRequest.getRequest(), QueryType.PPL)) {
139166
if (transformedRequest.isExplainRequest()) {
140167
unifiedQueryHandler.explain(
141168
transformedRequest.getRequest(),

0 commit comments

Comments
 (0)