fix(health): isolate transient dependency outages#82
Conversation
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughQdrant initialization now uses persisted readiness states and scheduled retries, with endpoint-specific collection validation and expanded unavailable-response handling. Qdrant initialization health gates the indicator, while actuator readiness and external dependency health are exposed separately. ChangesQdrant initialization and health separation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
bug
There was a problem hiding this comment.
Pull request overview
This PR updates the app’s health model to prevent transient external dependency outages (Qdrant / embedding provider) from blocking readiness, while still exposing dependency failures via a dedicated actuator health group.
Changes:
- Split health groups so
/actuator/health/readinessreflects only operational readiness state, while/actuator/health/dependenciesreports Qdrant + embedding keep-alive status. - Add retryable/pending Qdrant index initialization behavior to avoid restart loops during transport outages.
- Update security + tests + container healthcheck expectations to align with the new readiness/dependencies separation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/williamcallahan/javachat/JavaChatApplicationTests.java | Updates integration tests to assert readiness stays OK while dependency group remains observable and failing. |
| src/test/java/com/williamcallahan/javachat/config/QdrantIndexInitializerTest.java | Adds focused tests for distinguishing “confirmed missing” vs “transport unavailable” behavior during Qdrant initialization. |
| src/test/java/com/williamcallahan/javachat/config/QdrantHealthIndicatorTest.java | Extends health-indicator unit tests to account for index-initialization gating. |
| src/main/resources/application.properties | Moves Qdrant + embedding indicators out of readiness into a new dependencies health group. |
| src/main/java/com/williamcallahan/javachat/config/SecurityConfig.java | Permits unauthenticated access to /actuator/health/dependencies alongside existing allowed actuator endpoints. |
| src/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.java | Implements pending/failed initialization state with scheduled retries on transient transport failures. |
| src/main/java/com/williamcallahan/javachat/config/QdrantHealthIndicator.java | Incorporates initializer state into Qdrant health reporting. |
| Dockerfile | Aligns container HEALTHCHECK intent with readiness-only cutover behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Ensures Qdrant collections and indexes exist and match the configured embedding model. | ||
| * | ||
| * <p>This initializer is intentionally strict: | ||
| * - If a collection is reachable and mismatched (dimensions / hybrid config), startup fails. | ||
| * - If payload index creation is enabled and cannot be applied, startup fails. | ||
| * | ||
| * <p>This behavior prevents silent ingestion/retrieval failures when switching embedding providers | ||
| * or adopting hybrid (dense + sparse) collection schemas.</p> | ||
| * Ensures Qdrant collections and indexes exist and match the configured embedding model. Transport | ||
| * <p>Outages leave initialization pending and are retried without converting an unknown | ||
| * collection state into a create request. This prevents restart loops during DNS or network incidents | ||
| * while reachable configuration and schema defects remain strict failures.</p> |
| QdrantIndexInitializer indexInitializer = indexInitializerProvider.getIfAvailable(); | ||
| if (indexInitializer != null) { | ||
| Health initializationHealth = indexInitializer.initializationHealth(); | ||
| if (!Status.UP.equals(initializationHealth.getStatus())) { | ||
| return initializationHealth; | ||
| } | ||
| } | ||
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8849f9bc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (unavailableFailure != null) { | ||
| throw unavailableFailure; |
There was a problem hiding this comment.
With the default non-TLS configuration (QDRANT_PORT=6334), candidateRestBaseUrls() probes both the REST port 6333 and the raw configured port 6334; 6334 is normally Qdrant's gRPC port, not an HTTP REST endpoint. On a fresh Qdrant where the collection is absent, the 6333 probe returns 404 but the 6334 HTTP probe fails, so this branch throws QdrantUnavailableException and createHybridCollection() never runs; the scheduled retry repeats the same probes and app.qdrant.ensure-collections=true can remain pending indefinitely. Treat failures from the raw/fallback probe differently once a valid REST candidate has confirmed absence, or avoid probing the gRPC port for create decisions.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.java (2)
200-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
"/collections/"path segment duplicated 4x.Same literal repeated in
discoverCollectionEndpoint,createHybridCollection,fetchCollectionInfo, andensurePayloadIndex. Worth a shared constant (e.g.COLLECTIONS_PATH = "/collections/") to avoid drift if the API path ever changes.As per coding guidelines, "No inline numbers... or strings; define named constants (Magic Literals)."
Also applies to: 254-254, 311-311, 436-436
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.java` at line 200, Define a shared named constant for the “/collections/” path segment in QdrantIndexInitializer, then reuse it in discoverCollectionEndpoint, createHybridCollection, fetchCollectionInfo, and ensurePayloadIndex instead of repeating the inline string.Source: Coding guidelines
192-228: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant round trip: existence probe discards the body that
fetchCollectionInfore-fetches.
discoverCollectionEndpointdoesGET /collections/{name}purely to check existence (result discarded), thenvalidateCollections/fetchCollectionInfodoes a secondGETto the same URL just to read the body. That's 2x the Qdrant round trips per collection on every startup attempt and every 30s retry while pending.Consider having
discoverCollectionEndpointcapture and return the parsed body when it succeeds (e.g. add aJsonNode collectionInfofield toCollectionEndpoint), sovalidateCollectionscan reuse it instead of firing a second GET.Also applies to: 279-325
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.java` around lines 192 - 228, The collection existence check in discoverCollectionEndpoint currently discards the successful GET response, causing validateCollections/fetchCollectionInfo to request the same collection again. Update CollectionEndpoint and discoverCollectionEndpoint to retain the parsed collection response on success, then make validateCollections or fetchCollectionInfo reuse that data while preserving the existing probing, failure classification, and missing-collection behavior.src/main/java/com/williamcallahan/javachat/config/QdrantHealthIndicator.java (1)
40-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winJavadoc doesn't mention the new initializer-gating branch.
The method now short-circuits to
indexInitializer.initializationHealth()whenever an initializer is present and not UP (lines 53-59), but the doc above still only describes delegating to the external service monitor. Worth a line noting that collection initialization gates the result before the external snapshot is consulted, so future readers don't miss the branch.As per coding guidelines, Javadoc should focus on "why" and reflect actual behavior ("Why > What").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/williamcallahan/javachat/config/QdrantHealthIndicator.java` around lines 40 - 59, Update the Javadoc for QdrantHealthIndicator.health() to explain that collection initialization is checked first and a non-UP initialization status gates the returned health before the external service health snapshot is consulted. Keep the existing description of healthy and unhealthy outcomes accurate.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.java`:
- Around line 230-263: Refactor createHybridCollection and ensurePayloadIndex to
avoid exceeding the four-parameter limit by introducing and threading a
CollectionRequestContext containing baseUrl, collection, and headers; retain
only the method-specific vector names and dimensions as needed. Update all
callers to construct and pass the context, preserving the existing request
behavior and error handling.
- Around line 34-39: Fix the Javadoc summary for QdrantIndexInitializer so its
first sentence is complete and ends after describing that collections and
indexes match the configured embedding model. Move “Transport” into the
following paragraph so it begins as “Transport outages...” and preserve the
remaining documentation.
---
Nitpick comments:
In
`@src/main/java/com/williamcallahan/javachat/config/QdrantHealthIndicator.java`:
- Around line 40-59: Update the Javadoc for QdrantHealthIndicator.health() to
explain that collection initialization is checked first and a non-UP
initialization status gates the returned health before the external service
health snapshot is consulted. Keep the existing description of healthy and
unhealthy outcomes accurate.
In
`@src/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.java`:
- Line 200: Define a shared named constant for the “/collections/” path segment
in QdrantIndexInitializer, then reuse it in discoverCollectionEndpoint,
createHybridCollection, fetchCollectionInfo, and ensurePayloadIndex instead of
repeating the inline string.
- Around line 192-228: The collection existence check in
discoverCollectionEndpoint currently discards the successful GET response,
causing validateCollections/fetchCollectionInfo to request the same collection
again. Update CollectionEndpoint and discoverCollectionEndpoint to retain the
parsed collection response on success, then make validateCollections or
fetchCollectionInfo reuse that data while preserving the existing probing,
failure classification, and missing-collection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da7111db-3bc9-44c0-bdf6-c63ac444c768
📒 Files selected for processing (8)
Dockerfilesrc/main/java/com/williamcallahan/javachat/config/QdrantHealthIndicator.javasrc/main/java/com/williamcallahan/javachat/config/QdrantIndexInitializer.javasrc/main/java/com/williamcallahan/javachat/config/SecurityConfig.javasrc/main/resources/application.propertiessrc/test/java/com/williamcallahan/javachat/JavaChatApplicationTests.javasrc/test/java/com/williamcallahan/javachat/config/QdrantHealthIndicatorTest.javasrc/test/java/com/williamcallahan/javachat/config/QdrantIndexInitializerTest.java
Promotes dev commit a8849f9 after its completed dev deployment and successful readiness/dependency checks.