Mirror upstream elastic/elasticsearch#133523 for AI review (snapshot of HEAD tree) #91
Conversation
BASE=04ea11bf39e125a3a1a9f7c35e2854dd8f4ba3a5 HEAD=25ca0a65fb995b4d2535a24476d9cc4ae55469dd Branch=main
WalkthroughAdds periodic Java EA CI pipelines and generation logic, including BWC matrices and assorted test groups. Updates Backstage catalog with a new pipeline resource. Broadens Java version handling for vector modules and SIMD vectorization. Bumps ASM dependencies to 9.8 in two Gradle modules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Gradle task: updateCIBwcVersions
participant Tpl as Templates (.buildkite/pipelines/*.template.yml)
participant Gen as Generator (build.gradle logic)
participant BK as Output: periodic-java-ea.yml
Dev->>Gen: Run updateCIBwcVersions
Gen->>Tpl: Read periodic-java-ea.template.yml
Gen->>Tpl: Read periodic-java-ea.bwc.template.yml
Gen->>Gen: Inject BWC_LIST and BWC_STEPS
Gen-->>BK: Write .buildkite/pipelines/periodic-java-ea.yml
Note right of BK: File includes BWC matrices, test groups, retries, agents
sequenceDiagram
autonumber
participant App as ESVectorizationProvider.lookup(...)
participant JVM as Runtime/VM Info
participant Mod as Vector Module Loader
participant Prov as Provider
App->>JVM: Check feature version and VM
alt feature <= 25 and HotSpot
App->>Mod: Try enable vector module
alt Module available
App-->>Prov: Use Panama-based provider
else
App-->>Prov: Fallback Default provider
end
else
App-->>Prov: Default provider
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.buildkite/pipelines/periodic-java-ea.bwc.template.yml(1 hunks).buildkite/pipelines/periodic-java-ea.template.yml(1 hunks).buildkite/pipelines/periodic-java-ea.yml(1 hunks)build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/precommit/ThirdPartyAuditTask.java(2 hunks)build.gradle(1 hunks)catalog-info.yaml(1 hunks)distribution/tools/plugin-cli/build.gradle(1 hunks)libs/plugin-scanner/build.gradle(1 hunks)libs/simdvec/src/main21/java/org/elasticsearch/simdvec/internal/vectorization/ESVectorizationProvider.java(1 hunks)
🔇 Additional comments (3)
libs/simdvec/src/main21/java/org/elasticsearch/simdvec/internal/vectorization/ESVectorizationProvider.java (1)
53-53: Approve extending Panama Vector API support through Java 25The Vector API is included in JDK 25 EA as JEP 508, so this change correctly aligns with upstream and requires no further EA build verification.
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/precommit/ThirdPartyAuditTask.java (2)
64-65: LGTM! Imports follow the established pattern.The static imports for
VERSION_25andVERSION_26are consistent with the existing approach for earlier Java versions.
347-347: LGTM! Comment accurately reflects the incremental enablement strategy.The updated comment clarifies that vector module enablement is managed per-release rather than as a fixed set, improving maintainability.
| if (isJavaVersion(VERSION_20) | ||
| || isJavaVersion(VERSION_21) | ||
| || isJavaVersion(VERSION_22) | ||
| || isJavaVersion(VERSION_23) | ||
| || isJavaVersion(VERSION_24)) { | ||
| || isJavaVersion(VERSION_24) | ||
| || isJavaVersion(VERSION_25) | ||
| || isJavaVersion(VERSION_26)) { | ||
| spec.jvmArgs("--add-modules", "jdk.incubator.vector"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify alignment with ESVectorizationProvider.java runtime check.
The logic correctly extends vector module support to Java 25 and 26. However, the context mentions that ESVectorizationProvider.java broadens its runtime check from <= 24 to <= 25. Please verify that:
- The version ranges are intentionally different (this file includes 26, ESVectorizationProvider includes only up to 25)
- The discrepancy, if intentional, is documented or justified
Run the following script to examine the version check in ESVectorizationProvider.java:
Optional: Consider refactoring the version check pattern.
The conditional now checks 7 specific Java versions with OR conditions. As more versions are added, consider refactoring to a range check:
JavaVersion currentVersion = getRuntimeJavaVersion().orElse(
JavaVersion.toVersion(VersionProperties.getBundledJdkMajorVersion())
);
if (currentVersion.compareTo(VERSION_20) >= 0 && currentVersion.compareTo(VERSION_26) <= 0) {
spec.jvmArgs("--add-modules", "jdk.incubator.vector");
}🏁 Script executed:
#!/bin/bash
# Description: Verify the Java version check in ESVectorizationProvider.java
# Search for version comparisons in ESVectorizationProvider.java
rg -nP --type=java -C5 'ESVectorizationProvider' -A 20 | rg -P '(VERSION_2[456]|<=\s*2[456])'Length of output: 145
Align Java version checks for vector module support
- Resolve the mismatch: ThirdPartyAuditTask.java enables the vector module for JDK 26, but ESVectorizationProvider.java only checks
runtimeVersion <= 25(line 53). Extend that check to<= 26or remove JDK 26 here; document the rationale. - Optional: simplify the OR chain into a range comparison (e.g.,
currentVersion.compareTo(VERSION_20) >= 0 && <= VERSION_26).
🤖 Prompt for AI Agents
In
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/precommit/ThirdPartyAuditTask.java
around lines 348 to 356 there is a guard that enables the jdk.incubator.vector
module for JDK 20–26, but ESVectorizationProvider.java only treats
runtimeVersion <= 25 as vector-enabled; align the checks by updating
ESVectorizationProvider to include VERSION_26 (i.e., change the <= 25 check to
<= 26) or alternatively remove/roll back the JDK 26 case here — preferably
extend the provider to <= 26 and add a brief comment documenting the rationale;
while there, replace the long OR chain in this file with a range comparison
(e.g., check currentVersion >= VERSION_20 && <= VERSION_26) for clarity.
Single commit with tree=25ca0a65fb995b4d2535a24476d9cc4ae55469dd^{tree}, parent=04ea11bf39e125a3a1a9f7c35e2854dd8f4ba3a5. Exact snapshot of upstream PR head. No conflict resolution attempted.
Summary by CodeRabbit