Added Gating so VectorScorerInt4BenchmarkTests and VectorScorerInt4BulkBenchmarkTests Ignored on Java 21 by john-wagster · Pull Request #145060 · elastic/elasticsearch

john-wagster · 2026-03-27T03:04:33Z

Fixes: #145054
Fixes: #145055
Fixes: #145056
Fixes: #145057
Fixes: #145058

…ava 22

elasticsearchmachine · 2026-03-27T03:04:57Z

Pinging @elastic/es-search-relevance (Team:Search Relevance)

Copilot

Pull request overview

This PR addresses intermittent CI failures on Java 21 for the int4 vector scorer benchmark test suites by gating execution based on heap-segment support (JDK >= 22), avoiding code paths that currently NPE due to a null queryScorer.

Changes:

Added supportsHeapSegments() gating to avoid constructing/running Lucene/native int4 benchmark scorers when heap segments aren’t supported.
Conditionalized assertions and teardown for Lucene/native implementations in int4 benchmark tests (single and bulk).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
benchmarks/src/test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BenchmarkTests.java	Adds heap-segment gating around Lucene/native bench setup/assertions in score and query-score tests.
benchmarks/src/test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BulkBenchmarkTests.java	Adds heap-segment gating around Lucene/native bench setup/assertions across sequential/random and query/bulk query tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T03:09:03Z

.../test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BulkBenchmarkTests.java

    public void testQueryRandomBulk() throws Exception {
+
        for (int i = 0; i < 100; i++) {
            var vectorData = new VectorScorerInt4BulkBenchmark.VectorData(dims, 1000, 200);
            var scalar = createBench(VectorImplementation.SCALAR, vectorData);
-            var lucene = createBench(VectorImplementation.LUCENE, vectorData);
-            var nativeBench = createBench(VectorImplementation.NATIVE, vectorData);
+            VectorScorerInt4BulkBenchmark lucene = null;
+            VectorScorerInt4BulkBenchmark nativeBench = null;
+            if (supportsHeapSegments()) {
+                lucene = createBench(VectorImplementation.LUCENE, vectorData);
+                nativeBench = createBench(VectorImplementation.NATIVE, vectorData);
+            }

            try {
                float[] expected = scalar.scoreQueryMultipleRandomBulk();
-                assertArrayEquals("LUCENE queryRandomBulk", expected, lucene.scoreQueryMultipleRandomBulk(), delta);
-                assertArrayEquals("NATIVE queryRandomBulk", expected, nativeBench.scoreQueryMultipleRandomBulk(), delta);
+                if (supportsHeapSegments()) {
+                    assertArrayEquals("LUCENE queryRandomBulk", expected, lucene.scoreQueryMultipleRandomBulk(), delta);
+                    assertArrayEquals("NATIVE queryRandomBulk", expected, nativeBench.scoreQueryMultipleRandomBulk(), delta);
+                }


For the query-scoring tests (e.g., testQueryRandomBulk()), the method still runs when supportsHeapSegments() is false but then skips all lucene/native assertions and setup. This doesn't mark the test as ignored/skipped on Java 21 and still does the scalar work inside the loop. Consider adding assumeTrue("Only test with heap segments", supportsHeapSegments()) at the start of query-scoring test methods (as done in other scorer benchmark tests) and removing the repeated per-iteration conditional checks.

Copilot · 2026-03-27T03:09:04Z

.../src/test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BenchmarkTests.java

+import static org.elasticsearch.benchmark.vector.scorer.BenchmarkUtils.supportsHeapSegments;
+
 public class VectorScorerInt4BenchmarkTests extends ESTestCase {


PR title/description says these tests are "ignored on Java 21", but this change only conditionally skips creating lucene/native benches and assertions; the test methods still execute and will report as passing rather than skipped on Java 21. If the intent is to truly ignore/skip on Java 21, consider using an assumption (assumeTrue/assumeFalse) or a JUnit ignore/conditional mechanism so CI reflects the tests as skipped.

Copilot · 2026-03-27T03:09:04Z

.../src/test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BenchmarkTests.java

    public void testScores() throws Exception {
        for (int i = 0; i < 100; i++) {
            var data = new VectorScorerInt4Benchmark.VectorData(dims);
            var scalar = createBench(VectorImplementation.SCALAR, data);
-            var lucene = createBench(VectorImplementation.LUCENE, data);
-            var nativeBench = createBench(VectorImplementation.NATIVE, data);
+            VectorScorerInt4Benchmark lucene = null;
+            VectorScorerInt4Benchmark nativeBench = null;
+            if (supportsHeapSegments()) {
+                lucene = createBench(VectorImplementation.LUCENE, data);
+                nativeBench = createBench(VectorImplementation.NATIVE, data);
+            }


In testScores(), lucene/native benches are now only created when supportsHeapSegments() is true. But VectorScorerInt4Benchmark.setup() always initializes scorer for LUCENE/NATIVE and only gates queryScorer behind supportsHeapSegments(), so score() comparisons should still be valid on Java 21. This change reduces test coverage for LUCENE/NATIVE on Java 21 and doesn't actually "ignore" the test—only skips assertions. Consider always running the score() comparisons, and gate only the query-scoring tests (e.g., via assumeTrue("Only test with heap segments", supportsHeapSegments())).

Copilot · 2026-03-27T03:09:04Z

.../src/test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BenchmarkTests.java

    public void testQueryScores() throws Exception {
        for (int i = 0; i < 100; i++) {
            var data = new VectorScorerInt4Benchmark.VectorData(dims);
            var scalar = createBench(VectorImplementation.SCALAR, data);
-            var lucene = createBench(VectorImplementation.LUCENE, data);
-            var nativeBench = createBench(VectorImplementation.NATIVE, data);
+            VectorScorerInt4Benchmark lucene = null;
+            VectorScorerInt4Benchmark nativeBench = null;
+            if (supportsHeapSegments()) {
+                lucene = createBench(VectorImplementation.LUCENE, data);
+                nativeBench = createBench(VectorImplementation.NATIVE, data);
+            }

            try {
                float expected = scalar.scoreQuery();
-                assertEquals("LUCENE scoreQuery", expected, lucene.scoreQuery(), delta);
-                assertEquals("NATIVE scoreQuery", expected, nativeBench.scoreQuery(), delta);
+                if(supportsHeapSegments()) {
+                    assertEquals("LUCENE scoreQuery", expected, lucene.scoreQuery(), delta);
+                    assertEquals("NATIVE scoreQuery", expected, nativeBench.scoreQuery(), delta);
+                }


testQueryScores() currently runs even when supportsHeapSegments() is false, but then performs no lucene/native assertions (and still loops 100 times). Other scorer benchmark tests skip query-scoring tests entirely when heap segments aren't supported (e.g., VectorScorerFloatBenchmarkTests.java:69). Consider adding an assumeTrue at the start of this method so it is reported as skipped on Java 21 and avoids unnecessary work.

Copilot · 2026-03-27T03:09:04Z

.../src/test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BenchmarkTests.java

                float expected = scalar.score();
-                assertEquals("LUCENE score", expected, lucene.score(), delta);
-                assertEquals("NATIVE score", expected, nativeBench.score(), delta);
+                if(supportsHeapSegments()) {
+                    assertEquals("LUCENE score", expected, lucene.score(), delta);
+                    assertEquals("NATIVE score", expected, nativeBench.score(), delta);
+                }
            } finally {
                scalar.teardown();
-                lucene.teardown();
-                nativeBench.teardown();
+                if(supportsHeapSegments()) {
+                    lucene.teardown();


There are several if(supportsHeapSegments()) blocks without a space after if. This is inconsistent with the surrounding style in this file (e.g., if (supportsHeapSegments())) and may fail automated formatting/style checks. Please update to if (supportsHeapSegments()) consistently.

Copilot · 2026-03-27T03:09:05Z

.../test/java/org/elasticsearch/benchmark/vector/scorer/VectorScorerInt4BulkBenchmarkTests.java

    public void testSequential() throws Exception {
        for (int i = 0; i < 100; i++) {
            var vectorData = new VectorScorerInt4BulkBenchmark.VectorData(dims, 1000, 200);
            var scalar = createBench(VectorImplementation.SCALAR, vectorData);
-            var lucene = createBench(VectorImplementation.LUCENE, vectorData);
-            var nativeBench = createBench(VectorImplementation.NATIVE, vectorData);
+            VectorScorerInt4BulkBenchmark lucene = null;
+            VectorScorerInt4BulkBenchmark nativeBench = null;
+            if (supportsHeapSegments()) {
+                lucene = createBench(VectorImplementation.LUCENE, vectorData);
+                nativeBench = createBench(VectorImplementation.NATIVE, vectorData);
+            }

            try {
                float[] expected = scalar.scoreMultipleSequential();
-                assertArrayEquals("LUCENE sequential", expected, lucene.scoreMultipleSequential(), delta);
-                assertArrayEquals("NATIVE sequential", expected, nativeBench.scoreMultipleSequential(), delta);
+                if (supportsHeapSegments()) {
+                    assertArrayEquals("LUCENE sequential", expected, lucene.scoreMultipleSequential(), delta);
+                    assertArrayEquals("NATIVE sequential", expected, nativeBench.scoreMultipleSequential(), delta);
+                }


The new supportsHeapSegments() gating is applied to sequential/random/sequentialBulk/randomBulk tests too. However, in VectorScorerInt4BulkBenchmark.setup(), scorer for LUCENE/NATIVE is always initialized and only queryScorer is gated behind supportsHeapSegments(), so these non-query tests should still be comparable on Java 21. Consider keeping lucene/native coverage for the non-query scoring tests and only skipping the query-scoring tests on Java 21.

ldematte

I think copilot is right; we should just skip the whole thing if on JDK 21.
We can just assumeTrue(supportsHeapSegments(), ..) in a static method.

ldematte · 2026-03-27T14:38:51Z

If you want, I fixed the benchmarks as part of #145096

john-wagster · 2026-03-27T14:44:54Z

closed in favor of: #145096

added gating so the tests don't run for native and lucene unless on j…

88aa050

…ava 22

john-wagster requested review from Copilot, kkharbas and ldematte March 27, 2026 03:04

john-wagster added >test Issues or PRs that are addressing/adding tests :Search Relevance/Vectors Vector search v9.4.0 labels Mar 27, 2026

elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Mar 27, 2026

Copilot started reviewing on behalf of john-wagster March 27, 2026 03:05 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

[CI] Auto commit changes from spotless

d73bb3b

ldematte requested changes Mar 27, 2026

View reviewed changes

john-wagster closed this Mar 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Gating so VectorScorerInt4BenchmarkTests and VectorScorerInt4BulkBenchmarkTests Ignored on Java 21#145060

Added Gating so VectorScorerInt4BenchmarkTests and VectorScorerInt4BulkBenchmarkTests Ignored on Java 21#145060
john-wagster wants to merge 2 commits intoelastic:mainfrom
john-wagster:vectorscorerint4benchmarktests_gating_on_java22

john-wagster commented Mar 27, 2026

Uh oh!

elasticsearchmachine commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

ldematte left a comment

Uh oh!

ldematte commented Mar 27, 2026

Uh oh!

john-wagster commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		import static org.elasticsearch.benchmark.vector.scorer.BenchmarkUtils.supportsHeapSegments;

		public class VectorScorerInt4BenchmarkTests extends ESTestCase {

Conversation

john-wagster commented Mar 27, 2026

Uh oh!

elasticsearchmachine commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

ldematte left a comment

Choose a reason for hiding this comment

Uh oh!

ldematte commented Mar 27, 2026

Uh oh!

john-wagster commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants