Switch sandbox deps to analytics-api (JDK 21 surface)#5426
Open
bowenlan-amzn wants to merge 3 commits intoopensearch-project:feature/mustang-ppl-integrationfrom
Open
Conversation
Contributor
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit de41ce1.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
bowenlan-amzn
commented
May 8, 2026
4deb644 to
64ffed9
Compare
7 tasks
Merged
3 tasks
e2f6796 to
60cb52c
Compare
c8248a4 to
d769c12
Compare
Replace the three vendored binaries under libs/ with build-time downloads: - analytics-framework.jar / analytics-engine.jar: maven coordinates against https://ci.opensearch.org/ci/dbc/snapshots/maven (already declared as a subproject repo) → resolved as `org.opensearch.sandbox:analytics-{framework, engine}:3.7.0-SNAPSHOT`. - analytics-engine.zip: the snapshot maven repo doesn't publish the plugin distribution zip, so we fetch it from the feature-build URL via a new root task `:downloadAnalyticsEngineZip` (de.undercouch.download — same plugin the integ-test and doctest projects already use). Output lands in ${rootProject.buildDir}/distributions/analytics-engine-3.7.0-SNAPSHOT.zip and the existing `analyticsEngineZip` ext property points at it. The -PanalyticsEngineZip=/path override still works and skips the download. Wiring in the three subprojects that consume the zip (plugin, integ-test, doctest) ensures every test-cluster-bearing task (RestIntegTestTask, StandaloneRestIntegTestTask, RunTask) dependsOn the download task before the cluster boots. mavenLocal() now excludes the org.opensearch.sandbox group: a locally- published OpenSearch core build can install these artifacts with Gradle Module Metadata declaring a JVM version different from the published snapshot, which can shadow the remote and break resolution. Always pull sandbox artifacts from the CI snapshot repo. Why now: - libs/ adds 17 MB of binaries to every clone. - The vendored zip drifts every time analytics-engine bumps; remote URL always points at the latest feature-build artifact. - Aligns with the OpenSearch convention of consuming sandbox artifacts via the published snapshot repo (same as opensearch-core, opensearch- job-scheduler-spi, etc., already wired this way). NOT YET COMPILABLE: the currently-published analytics-framework snapshot (3.7.0-20260409.054801-56) is older than what was vendored in libs/ and exposes a sync `Stream execute(LogicalPlan, Object)` instead of the async `void execute(LogicalPlan, Object, ActionListener<Stream>)` that SQL's AnalyticsExecutionEngine targets. Wait for upstream to publish a fresh sandbox snapshot before merging. Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
d769c12 to
ef92d85
Compare
Without this, RestIntegTestTask subclasses in integ-test don't have opensearch-agent.jar on their JVM classpath, causing AgentPolicy class initialization to fail at test startup. Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
ef92d85 to
de41ce1
Compare
ahkcs
approved these changes
May 9, 2026
RyanL1997
approved these changes
May 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sql depends on two interfaces from analytics-engine:
QueryPlanExecutorandOpenSearchSchemaBuilder. Previously these were vendored as local JARs. This PR:analytics-api— a new JDK-21-targeted Maven module that exposes only those two interfaces. Depends on Extract analytics-api module for JDK 21 downstream consumers OpenSearch#21574.integTestruns without it.analyticsEngineCompatIT— a dedicated smoke test that boots sql alongside analytics-engine + arrow-flight-rpc to verify the three plugins load together without classloader errors.Root cause of the original JDK mismatch
analytics-frameworkandanalytics-enginetarget JDK 25 (they usejava.lang.foreign). sql targets JDK 21. Consuming their JARs directly causedclass file has wrong version 69.0, should be 65.0on the JDK-21 CI matrix. The fix isanalytics-api: a thin module containing only the interfaces sql needs, compiled at JDK 21.Why
analytics-apiisimplementation(notcompileOnly) in pluginOpenSearch scans plugin classes at startup by calling
getMethods(). sql's handler classes referenceQueryPlanExecutorin their method signatures. Withoutanalytics-apibundled in the plugin zip, OpenSearch throwsNoClassDefFoundError: QueryPlanExecutorat startup even when no analytics-engine code path runs. When analytics-engine IS installed, its classloader provides the class viaextendedPluginsparent-first delegation — the bundled copy is unused but harmless.Verification
Tested locally with
analytics-apipublished tomavenLocalfrom the OpenSearch PR branch:./gradlew :core:compileJava— BUILD SUCCESSFUL./gradlew :opensearch-sql-plugin:bundlePlugin— BUILD SUCCESSFUL./gradlew :integ-test:analyticsEngineCompatIT— BUILD SUCCESSFUL (cluster boots, all three plugins load)analytics-api-3.7.0-SNAPSHOTis published to the snapshot Maven repo.Depends on
analytics-apimodule targeting JDK 21