Skip to content

Migrate NodeHealthAPI: flip delegation so V2 owns logic, V1 bridges to it#22

Draft
Copilot wants to merge 21 commits intomainfrom
copilot/migrate-node-health-api
Draft

Migrate NodeHealthAPI: flip delegation so V2 owns logic, V1 bridges to it#22
Copilot wants to merge 21 commits intomainfrom
copilot/migrate-node-health-api

Conversation

Copy link

Copilot AI commented Feb 22, 2026

HealthCheckHandler previously owned all health-check logic and NodeHealthAPI was a thin JAX-RS wrapper that delegated back to it. This inverts that relationship: the V2 NodeHealthAPI now owns the logic and HealthCheckHandler is the thin bridge.

Key changes

NodeHealthAPI — now owns all business logic

  • healthCheckCloudMode(), healthCheckLegacyMode(), isWithinGenerationLag(), findUnhealthyCores() moved here
  • Uses instantiateJerseyResponse(NodeHealthResponse.class) for proper Jersey context registration
  • Exposes checkNodeHealth(Boolean requireHealthyCores, Integer maxGenerationLag) for the V1 path to call
  • Strongly typed throughout (NodeStatus enum, no NamedList in business logic)

HealthCheckHandler — thin V1 bridge (~10 lines of logic)

V2ApiUtils.squashIntoSolrResponseWithoutHeader(
    rsp,
    new NodeHealthAPI(coreContainer).checkNodeHealth(requireHealthyCores, maxGenerationLag));
  • findUnhealthyCores() kept as a @Deprecated delegation shim pointing to NodeHealthAPI

Utils.getReflectWriter() — enum serialization fix

Enum values in a NamedList (e.g. NodeStatus.OK) previously fell through to ClassName:NAME serialization since enums have no @JsonProperty fields. Added an early-exit that returns ((Enum<?>) o).name() so they round-trip as plain strings through javabin.

Bug fixes in isWithinGenerationLag

The original condition generationDiff < maxGenerationLag was inverted — it treated healthy cores as lagging and vice versa. Corrected to > maxGenerationLag with return semantics true = within acceptable lag (healthy), false = lagging too far. Also fixed missing slf4j format arguments in the negative-diff warn call.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • develocity.apache.org
    • Triggering command: /usr/lib/jvm/temurin-21-jdk-amd64/bin/java /usr/lib/jvm/temurin-21-jdk-amd64/bin/java -XX:ReservedCodeCacheSize=256m -XX:TieredStopAtLevel=1 -XX:&#43;UseParallelGC -XX:ActiveProcessorCount=1 --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED (dns block)
  • metadata.google.internal
    • Triggering command: /usr/lib/jvm/temurin-21-jdk-amd64/bin/java /usr/lib/jvm/temurin-21-jdk-amd64/bin/java -Dcommon-solr.dir=/home/REDACTED/work/solr/solr/solr -Dgradle.lib.dir=/home/REDACTED/.gradle/wrapper/dists/gradle-8.10-bin/deqhafrv1ntovfmgh0nh3npr9/gradle-8.10/lib -Dgradle.user.home=/home/REDACTED/.gradle -Dgradle.worker.jar=/home/REDACTED/.gradle/caches/8.10/workerMain/gradle-worker.jar -Djava.awt.headless=true -Djava.security.egd=file:/dev/./urandom -Djava.security.manager=default -Djava.security.policy=/home/REDACTED/work/solr/solr/gradle/testing/randomization/policies/solr-tests.policy -Djava.util.logging.config.file=/home/REDACTED/work/solr/solr/gradle/testing/defaults-tests/logging.properties -Djdk.map.althashing.threshold=0 -Djetty.insecurerandom=1 -Djetty.testMode=1 -Djunit4.childvm.count=1 -Djunit4.childvm.id=0 -Dorg.gradle.internal.worker.tmpdir=/home/REDACTED/work/solr/solr/solr/core/build/tmp/test/work -DtempDir=/home/REDACTED/work/solr/solr/build/tmp/tests-tmp -Dtests.LUCENE_VERSION=10.3.2 -Dtests.asserts=true -Dtests.codec=random (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Copilot AI changed the title [WIP] Migrate NodeHealthAPI to JAX-RS annotations Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS annotations Feb 22, 2026
Copilot AI requested a review from epugh February 22, 2026 13:26
Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Copilot AI changed the title Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS annotations Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS; add mock-free integration tests Feb 22, 2026
…ef guide

Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Copilot AI changed the title Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS; add mock-free integration tests Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS; add ref guide link Feb 22, 2026
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 28, 2026
@epugh
Copy link
Owner

epugh commented Feb 28, 2026

Superseded by apache#4171 — retargeted to the upstream repo.

@epugh epugh closed this Feb 28, 2026
…kHandler delegates to V2

- NodeHealthAPI now owns all business logic (cloud mode, legacy mode,
  isWithinGenerationLag, findUnhealthyCores) using strongly-typed
  NodeHealthResponse / NodeStatus throughout.
- HealthCheckHandler becomes a thin V1 bridge: handleRequestBody()
  creates NodeHealthAPI(coreContainer).checkNodeHealth(...) and
  squashes the typed response into SolrQueryResponse.
- findUnhealthyCores() moved to NodeHealthAPI as a public static util;
  HealthCheckHandler keeps a @deprecated delegation shim so existing
  callers continue to compile.
- HealthCheckHandlerTest updated to call NodeHealthAPI.findUnhealthyCores()
  directly.
- Utils.getReflectWriter() now serialises Enum values as their .name()
  string so that NodeStatus.OK round-trips as "OK" through
  NamedList/javabin, keeping HealthCheckHandlerTest assertions passing.
- Fixed pre-existing bug in isWithinGenerationLag: condition was
  `generationDiff < maxGenerationLag` (wrong); corrected to
  `generationDiff > maxGenerationLag` with the return values adjusted
  so the method returns true=healthy / false=lagging-too-far.
- Fixed missing slf4j log arguments in the negative-diff warning.

Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Copilot AI changed the title Migrate NodeHealthAPI from homegrown @EndPoint to JAX-RS; add ref guide link Migrate NodeHealthAPI: flip delegation so V2 owns logic, V1 bridges to it Mar 9, 2026
Copilot finished work on behalf of epugh March 9, 2026 19:24
I used claude for this regressoin test and I don't love how verbose they are.  I tried a mock approach first and it was worse.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:api client:solrj documentation Improvements or additions to documentation tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants