Repository navigation
fix(server): add bounded dependency readiness without changing liveness - #1853
Open
DivyamTalwar wants to merge 1 commit into
Open
DivyamTalwar wants to merge 1 commit into
DivyamTalwar wants to merge 1 commit into
Conversation
Fixes MemMachine#1777 Implemented and reviewed with OpenAI Codex. Signed-off-by: Divyam Talwar <divyamtalwar0@gmail.com>
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the change
The current health endpoint stays healthy while storage is unreachable, so the repository's deployment probes can keep routing traffic to a worker whose memory operations fail. This adds dependency readiness while preserving the existing liveness response.
Description
GET /api/v2/ready: 200 after startup and successful required-storage checks; 503 during startup/shutdown, outages, timeouts, or unsupported required configurations./api/v2/healthkeeps its status and body./healthfor Kubernetes liveness.Fixes/Closes
Fixes #1777
Type of change
How Has This Been Tested?
Unit Test
Integration Test
Linux: 228 focused and adjacent tests passed, with four existing integration cases deselected. After the independent review's early-stop/restart fix, 60 readiness and lifecycle tests passed, including both new stop-before-start and early-start-failure/retry cases. The final focused file has 14 cases; all 14 also passed on macOS after the lifecycle correction.
macOS with Docker: one native integration test passed, no skips, and passed again on the final source after the lifecycle correction, using disposable PostgreSQL, Neo4j, and Qdrant containers. Both event and declarative projects perform real memory adds/searches. Pausing each dependency changes readiness to 503 while liveness remains 200; unpausing restores 200 and successful adds/searches, preserves old data, and retains the same serving client/engine/store objects.
The integration uses real application ASGI routing and native storage clients, rather than a separately listening HTTP server. Only the external embedding provider is replaced by the repository's deterministic
FakeEmbedder; the identity reranker is local and real. Containers, ports, passwords, and data are test-owned; cleanup targets those exact containers.Ruff check and formatting pass for all changed Python files;
typasses for changed server modules;git diff --checkand patch checks pass. Helm lint and four renders pass: default/event, declarative, external stores, and the older-image compatibility override. Compose's healthcheck structure and five-second curl bound were checked locally.Independent read-only review found the early-stop/restart edge above; it was fixed, regressed, and re-reviewed. No further blocking findings were reported. Existing Testcontainers/Starlette deprecation warnings remain.
Reproduce the real storage test from the repository root:
Deployment compatibility and limits
The Helm/Compose readiness default now requires an application image containing
/api/v2/ready. Upgrade the image before or together with the probe change. Older images can explicitly selectmemmachine.readiness.path=/api/v2/healthorMEMMACHINE_READINESS_PATH=/api/v2/health; that mode checks liveness only. Old images with the new path remain visibly unready with 404. Roll back image and probe configuration together. Helm chart version increases to 0.3.0; application release metadata is not changed.Readiness checks lightweight storage reachability/read access, not embedding/LLM providers, migrations, write permissions, or collection/index correctness. Cold Milvus Lite reports
unsupportedbecause creating its client would start a local server. Native outage/recovery evidence covers PostgreSQL, Neo4j, and Qdrant; no live NebulaGraph or remote Milvus result is claimed.Checklist
Maintainer Checklist
Further comments
Implemented with OpenAI Codex and independently reviewed by a second coding agent. The final source was checked against the reviewed file manifest before submission. The GitHub-verified commit signature and Signed-off-by trailer were checked before opening this PR.