SOLR-16458: Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS by epugh · Pull Request #4171 · apache/solr

epugh · 2026-02-28T13:08:36Z

Migrates NodeHealthAPI — the last node-level V2 API still using Solr's homegrown @EndPoint annotation — to standard JAX-RS, following the same pattern as NodeLogging, GetPublicKey, etc.

Design

The logic stays in HealthCheckHandler (minimising diff surface). NodeHealthAPI is a thin JAX-RS wrapper (~60 lines) that delegates entirely to it.

Key changes

solr/api — New NodeHealthApi interface (@Path, @GET, @Operation) and NodeHealthResponse model (status, message, num_cores_unhealthy)
NodeHealthAPI — Replaces @EndPoint with JAX-RS; injects CoreContainer, delegates to HealthCheckHandler
HealthCheckHandler — Logic unchanged; adds public NodeHealthResponse checkNodeHealth(Boolean, Integer) as the shared entry point for both v1 (handleRequestBody) and v2 (NodeHealthAPI); switches to getJerseyResources() / empty getApis()
V2NodeAPIMappingTest — Removes the now-obsolete @EndPoint/ApiBag routing test for health
NodeHealthAPITest — New Mockito unit tests for the API class
NodeHealthAPITest2 — New mock-free integration tests: cloud-mode via real MiniSolrCloudCluster, legacy mode via embedded CoreContainer built from NodeConfig
implicit-requesthandlers.adoc — Health section now links to both HealthCheckHandler (v1) and NodeHealthAPI (v2) javadocs

Co-authored-by: epugh <22395+epugh@users.noreply.github.com>

…ef guide Co-authored-by: epugh <22395+epugh@users.noreply.github.com>

epugh · 2026-02-28T13:21:36Z

Some outstanding questions: 1) there is a more mock and less mock versions of the same test. Which do we prefer? 2) Does this seem like a reasonable pattern for the conversion?

epugh · 2026-02-28T13:27:50Z

I wish I didnt' have TWO ways of writing tests, one for cloud and one for standalone... sigh.

dsmiley · 2026-02-28T18:47:35Z

solr/api/src/java/org/apache/solr/client/api/model/NodeHealthResponse.java

+  @JsonProperty public String message;
+
+  @JsonProperty("num_cores_unhealthy")
+  public Long numCoresUnhealthy;


Long isn't warranted; lets just do Integer.

this did start changing one of the existing HealthCheckHandler methods... Rerunning tests now.

Long is important - it allows the value to be null and omitted from responses entirely. That's not just a theoretical distinction either - the code in HealthCheckHandler sets this conditionally today!

if (unhealthyCores > 0) { rsp.add(STATUS, FAILURE); rsp.add("num_cores_unhealthy", unhealthyCores); rsp.setException( new SolrException( SolrException.ErrorCode.SERVICE_UNAVAILABLE, unhealthyCores + " out of " + coreContainer.getNumAllCores() + " replicas are currently initializing or recovering")); return; }

If @dsmiley has a good reason why it's not warranted in this case, then ignore me. But in general I've been using Long, Integer, Float, etc. in these Response objects any time that it's conceivable that a response might omit the value entirely, which seems true in this case.

Do you think I should back ouit the change to LOng... One thing I don't like is that the call to findUnhealthyCores now has a return Math.toIntExact which seems messy.

Do you think I should back ouit the change to LOng

Kindof, yes, that's why I mentioned it haha 😛. You're welcome to disagree of course, or maybe David will offer a reason why he thought it was unnecessary. But that's my 2c.

My only point was to use Integer and not Long. I wasn't arguing for a primitive or anything else.

what about the use of return Math.toIntExact? Makes me start to think that whatever else is returning longs should be converted to ints too?

Stream.count() returns a long. It doesn't mean that this particular stream would ever actually need a long.

solr/core/src/java/org/apache/solr/handler/admin/HealthCheckHandler.java

epugh · 2026-03-01T20:53:59Z

tests all pass!

gerlowskija · 2026-03-03T13:34:39Z

@epugh - this should probably be attached to one of the v2 JIRA tickets or another. Maybe https://issues.apache.org/jira/browse/SOLR-16458?

epugh · 2026-03-03T13:37:44Z

@epugh - this should probably be attached to one of the v2 JIRA tickets or another. Maybe https://issues.apache.org/jira/browse/SOLR-16458?

Sure... I suppose I could be crosslinking all of these to various JIRAs?

gerlowskija · 2026-03-03T13:47:22Z

changelog/unreleased/migrate-node-health-api.yml

@@ -0,0 +1,7 @@
+title: Migrate NodeHealthAPI to JAX-RS. NodeHealthAPI now has OpenAPI and SolrJ support.


[0] I'd try to focus this changelog more on what matters to the user. Specifically - what is the new SolrJ class for and where can a user find it?

i.e.

SolrJ now offers a SolrRequest class allowing users to perform single-node healthchecks: NodeApi.CheckNodeHealth

Something like that...

gerlowskija · 2026-03-03T13:47:58Z

changelog/unreleased/migrate-node-health-api.yml

@@ -0,0 +1,7 @@
+title: Migrate NodeHealthAPI to JAX-RS. NodeHealthAPI now has OpenAPI and SolrJ support.
+type: changed


[0] From a user's perspective - this is "added". There's a new SolrJ capability they didn't have before!

gerlowskija · 2026-03-03T13:48:13Z

changelog/unreleased/migrate-node-health-api.yml

+  - name: Eric Pugh
+links:
+- name: PR#4171
+  url: https://github.com/apache/solr/pull/4171


[0] Link to JIRA

gerlowskija · 2026-03-03T14:05:50Z

solr/api/src/java/org/apache/solr/client/api/endpoint/NodeHealthApi.java

+  @Operation(
+      summary = "Determine the health of a Solr node.",
+      tags = {"node"})
+  NodeHealthResponse checkNodeHealth(


[0] By default, the name of the generated SolrRequest class is: <tagVal>Api.<MethodName>.

We can override that if desired by specifying an operationId parameter in the @Operation annotation, in which case the pattern becomes: <tagVal>Api.<OperationId>

Do you like NodeApi.CheckNodeHealth as a classname, or would NodeApi.Healthcheck or something similar be better?

gerlowskija · 2026-03-03T14:07:26Z

solr/api/src/java/org/apache/solr/client/api/model/NodeHealthResponse.java

+/** Response body for the '/api/node/health' endpoint. */
+public class NodeHealthResponse extends SolrJerseyResponse {
+
+  @JsonProperty public String status;


[Q] Does status have certain expected values? If so, those can be documented using a @Schema annotation, or the whole field can be made an 'enum' which is probably even better.

gerlowskija · 2026-03-03T14:37:47Z

solr/core/src/test/org/apache/solr/handler/admin/api/NodeHealthAPITest.java

+ * mocks.
+ *
+ * <p>Cloud-mode tests use a real {@link org.apache.solr.cloud.MiniSolrCloudCluster} and get a
+ * {@link CoreContainer} directly from a {@link JettySolrRunner}. Legacy (standalone) mode tests


[Q] Do we have other tests that do standalone testing within a SolrCloudTestCase?

It feels weird conceptually. And in practical terms SolrCloudTestCase does some work that makes it much slower on a per-test basis than our other base classes. Doing standalone testing in a SolrCloudTestCase is going to end up paying that runtime cost for no reason.

gerlowskija · 2026-03-03T14:42:18Z

solr/core/src/test/org/apache/solr/handler/admin/api/NodeHealthAPITest.java

+  public void testCloudMode_RequireHealthyCoresReturnOkWhenAllCoresHealthy() {
+    CoreContainer coreContainer = cluster.getJettySolrRunner(0).getCoreContainer();
+
+    // requireHealthyCores=true should succeed on a node with no unhealthy cores


[-1] You haven't actually created any cores!!

Can you create a collection or something that'll cause this test to actually exercise the per-core logic currently in HealthcheckHandler?

gerlowskija · 2026-03-03T14:45:13Z

solr/core/src/test/org/apache/solr/handler/admin/api/NodeHealthAPITest.java

+import org.junit.Test;
+
+/**
+ * Integration tests for {@link NodeHealthAPI} that use real Solr instances instead of Mockito


[Q] If these are intended to be integration tests, why aren't they using the HTTP APIs the way a user would?

This would be an excellent place IMO to make use of the generated SolrRequest class. Doing so would kill two birds with one stone: making the tests more of a true integration test, and getting some validation on the SolrJ code as well.

gerlowskija · 2026-03-03T14:47:49Z

solr/core/src/test/org/apache/solr/handler/admin/api/NodeHealthAPITest.java

+    }
+  }
+
+  // ---- Legacy (standalone, non-ZooKeeper) mode tests ----


[Q] Are we calling this "Legacy" now? 🤔

OMG... remove this comment

gerlowskija · 2026-03-03T14:53:15Z

solr/solr-ref-guide/modules/configuration-guide/pages/implicit-requesthandlers.adoc

-v2: `api/node/health` |{solr-javadocs}/core/org/apache/solr/handler/admin/HealthCheckHandler.html[HealthCheckHandler] |
+v2: `api/node/health` |v1: {solr-javadocs}/core/org/apache/solr/handler/admin/HealthCheckHandler.html[HealthCheckHandler]
+
+v2: {solr-javadocs}/core/org/apache/solr/handler/admin/api/NodeHealthAPI.html[NodeHealthAPI] |


[Q] Is it the implementation Javadocs we want to point people to here, or would the solr/api interface docs be more helpful?

Or more broadly - is there much value even in pointing to either Javadoc on the v2 side? HealthcheckHandler has a nice good blurb, but neither NodeHealthAPI nor NodeHealthApi have much of anything that's worth pointing a user at IMO...

dsmiley · 2026-03-03T15:14:55Z

Just want to give kudos to Jason's thoroughly amazing code review. Really shows how important it is that we have the right reviewers on the right issues. Hopefully after this issue is done, similar issues can follow the same lessons/advise. Assuming LLM is doing the bulk of the work, it can be pointed specifically at this PR to learn the key aspects to successfully do similar transitions.

Copilot AI and others added 4 commits February 22, 2026 12:52

Initial plan

0585c2a

Migrate NodeHealthAPI from @endpoint to JAX-RS annotations

4aa9e8b

Co-authored-by: epugh <22395+epugh@users.noreply.github.com>

Add NodeHealthAPITest2: integration tests without Mockito mocks

8e2ccd5

Co-authored-by: epugh <22395+epugh@users.noreply.github.com>

Restore logic to HealthCheckHandler, simplify NodeHealthAPI, update r…

a50f682

…ef guide Co-authored-by: epugh <22395+epugh@users.noreply.github.com>

epugh mentioned this pull request Feb 28, 2026

Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS; add ref guide link epugh/solr#22

Closed

github-actions bot added documentation Improvements or additions to documentation tests cat:api labels Feb 28, 2026

epugh mentioned this pull request Feb 28, 2026

SOLR-16458: Node system info V2 api jersey resource #4078

Open

8 tasks

epugh marked this pull request as ready for review February 28, 2026 13:20

epugh requested review from gerlowskija and janhoy February 28, 2026 13:20

restore docs

b8159ab

epugh added the no-changelog label Feb 28, 2026

Copilot AI mentioned this pull request Feb 28, 2026

Migrate RenameCoreAPI from homegrown @EndPoint/@Command to JAX-RS; use SolrJettyTestRule in tests epugh/solr#24

Closed

lint

01696e3

dsmiley reviewed Feb 28, 2026

View reviewed changes

epugh added 2 commits March 1, 2026 12:58

Keep the non mock version

dac4e14

Responding to feedback

ec8252a

lint

b592322

epugh changed the title ~~Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS; add ref guide link~~ Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS Mar 2, 2026

epugh removed the no-changelog label Mar 3, 2026

add changelog

ca88d6e

epugh changed the title ~~Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS~~ SOLR-16458: Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS Mar 3, 2026

gerlowskija requested changes Mar 3, 2026

View reviewed changes

		@@ -0,0 +1,7 @@
		title: Migrate NodeHealthAPI to JAX-RS. NodeHealthAPI now has OpenAPI and SolrJ support.

		@@ -0,0 +1,7 @@
		title: Migrate NodeHealthAPI to JAX-RS. NodeHealthAPI now has OpenAPI and SolrJ support.
		type: changed

Conversation

epugh commented Feb 28, 2026

Design

Key changes

Uh oh!

epugh commented Feb 28, 2026

Uh oh!

epugh commented Feb 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

epugh commented Mar 1, 2026

Uh oh!

gerlowskija commented Mar 3, 2026

Uh oh!

epugh commented Mar 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants