Skip to content

fix: keep VectorDB health probes responsive - #2617

Open
KyleZheng1284 wants to merge 2 commits into
NVIDIA:mainfrom
KyleZheng1284:fix/vectordb-health-readiness
Open

fix: keep VectorDB health probes responsive#2617
KyleZheng1284 wants to merge 2 commits into
NVIDIA:mainfrom
KyleZheng1284:fix/vectordb-health-readiness

Conversation

@KyleZheng1284

Copy link
Copy Markdown
Collaborator

Description

Keep the VectorDB service responsive when LanceDB health inspection is slow or the document catalog is large.

This change:

  • Adds an unauthenticated GET /v1/live endpoint that performs no backend or storage operations.
  • Runs backend health and metrics inspection outside the asyncio event loop.
  • Changes LanceDB health inspection to query only pending recovery records using recovery_state != ''.
  • Adds an additive scalar index on recovery_state for existing and new catalogs.
  • Uses /v1/live for VectorDB startup and liveness probes while retaining /v1/health for readiness.
  • Documents the distinction between process liveness and storage-aware readiness.

The existing /v1/health response contract and status codes remain unchanged. This does not change ingestion, retrieval, ranking, identifiers, collection behavior, or stored document data, and it requires no data migration.

Validation

  • 118 focused and adjacent tests passed.
  • Repository pre-commit checks passed.
  • Helm lint and rendering passed.
  • Kubernetes server-side dry-run validation passed.
  • Direct HTTP smoke tests confirmed successful responses from /v1/live, /v1/health, and /v1/collections.
  • A local container build was not run because the Docker daemon was unavailable; container validation remains covered by CI.

Checklist

Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
@KyleZheng1284
KyleZheng1284 requested review from a team as code owners August 29, 2026 00:23
@KyleZheng1284
KyleZheng1284 requested a review from ChrisJar August 29, 2026 00:23
@greptile-apps

greptile-apps Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR separates shallow VectorDB liveness from storage-aware readiness and moves backend inspection off the asyncio event loop.

  • Adds the unauthenticated /v1/live endpoint and assigns it to VectorDB startup and liveness probes.
  • Filters LanceDB health inspection to pending recovery records and creates a scalar index for that state.
  • Offloads health and metrics inspection to worker threads and documents concurrent backend access expectations.
  • Updates focused service, catalog, Helm, and deployment documentation tests.

Confidence Score: 4/5

The PR is not yet safe to merge because backend health inspection can still race collection cleanup and transiently mark a healthy VectorDB unavailable.

Health inspection now runs concurrently on the shared backend, but LanceDB collection health reads cached table handles without the lock used when cleanup drops tables and removes those handles, leaving the previously reported readiness and metrics failure reachable.

Files Needing Attention: nemo_retriever/src/nemo_retriever/service/vectordb_app.py, nemo_retriever/src/nemo_retriever/common/vdb/lancedb_collections.py

Important Files Changed

Filename Overview
nemo_retriever/src/nemo_retriever/service/vectordb_app.py Adds shallow liveness and offloads backend inspection, but the previously reported synchronization defect remains in the shared backend path.
nemo_retriever/src/nemo_retriever/common/vdb/lancedb_collections.py Optimizes recovery health scans and adds an index, while collection health still accesses table state outside the mutation lock.
nemo_retriever/src/nemo_retriever/common/vdb/adt_vdb.py Documents the new concurrency expectation for backend implementations without enforcing it.
nemo_retriever/helm/templates/deployment-vectordb.yaml Correctly moves startup and liveness probes to the shallow endpoint while preserving readiness on /v1/health.
nemo_retriever/tests/test_service_vectordb_app.py Verifies liveness responsiveness during blocked health inspection but does not exercise overlapping real LanceDB health and mutation operations.
nemo_retriever/tests/test_lancedb_collections.py Covers filtered health scans and additive recovery-state index creation for existing catalogs.

Sequence Diagram

sequenceDiagram
    participant K as Kubernetes
    participant A as VectorDB API
    participant T as Worker thread
    participant L as LanceDB
    K->>A: GET /v1/live
    A-->>K: "200 {"status":"ok"}"
    K->>A: GET /v1/health
    A->>T: Offload backend health
    T->>L: Inspect catalog and storage
    L-->>T: Health details
    T-->>A: Backend health
    A-->>K: 200 or 503
Loading

Reviews (2): Last reviewed commit: "docs: clarify VectorDB health contracts" | Re-trigger Greptile

async def health() -> dict[str, Any]:
current = state
backend_health = _safe_backend_health(current)
backend_health = await asyncio.to_thread(_safe_backend_health, current)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unsynchronized backend health inspection

If health, metrics, reconciliation, writes, or queries overlap, asyncio.to_thread accesses the same stateful VectorDB instance from multiple threads without the query semaphore or collection-store write lock, exposing shared LanceDB handles, cache state, and health counters to inconsistent results or backend errors. Serialize these inspections or explicitly establish and document the backend's thread-safety contract.

Rule Used: Models and stateful components accessed from Ray a... (source)

Knowledge Base Used: Service observability and integrations

Prompt To Fix With AI
This is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/service/vectordb_app.py
Line: 456

Comment:
**Unsynchronized backend health inspection**

If health, metrics, reconciliation, writes, or queries overlap, `asyncio.to_thread` accesses the same stateful VectorDB instance from multiple threads without the query semaphore or collection-store write lock, exposing shared LanceDB handles, cache state, and health counters to inconsistent results or backend errors. Serialize these inspections or explicitly establish and document the backend's thread-safety contract.

**Rule Used:** Models and stateful components accessed from Ray a... ([source](https://github.com/nvidia/nemo-retriever/blob/ee27a78852a4643a1bddf318679184ab92a41684/nemo_retriever/.greptile/config.json))

**Knowledge Base Used:** [Service observability and integrations](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia/nemo-retriever/-/docs/service-observability-and-integrations.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment thread nemo_retriever/src/nemo_retriever/service/vectordb_app.py
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant