fix: evict stale GraphQL schema caches and bound test memory usage#9807
fix: evict stale GraphQL schema caches and bound test memory usage#9807fatih-acar wants to merge 5 commits into
Conversation
Merging this PR will degrade performance by 23.39%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_base_schema_duplicate_CoreProposedChange |
1.5 ms | 2.2 ms | -32.4% |
| ❌ | test_schemabranch_duplicate |
6.7 ms | 7.7 ms | -13.17% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fac/optimize-integration-tests-4oa27 (18a6cc7) with stable (2a8e5c8)
There was a problem hiding this comment.
No issues found across 6 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Modifies core GraphQL schema eviction logic; requires human review to verify correctness and avoid unintended side effects.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. Core registry logic and eviction strategy changes require human review to ensure no schema serving errors or memory leaks. High-impact refactor affecting API workers.
Re-trigger cubic
| self.evict_schema_hash(schema_hash=schema_hash) | ||
|
|
||
| for branch_name in inactive_branches: | ||
| self._branch_hash_activation_by_branch_name.pop(branch_name, None) |
There was a problem hiding this comment.
One thing to consider here is if a user searches using at then we might end up with another schema hash. I think there are problems with that now if the schema differs, at the very least if the old schema has node kinds that have been removed in the newer schema. It could be that we shouldn't consider that aspect just now but I think at some point we should have a timestamp for when the schema was created and perhaps only delete schemas if they are unused and older than 15 minutes or so. Not requesting a change only mentioning this.
The neo4j image auto-sizes its heap and page cache from host memory, so each xdist worker's container grows to 3-4GB regardless of the small test datasets. With several workers this overcommits the host and gets containers OOM-killed (measured: -n 8 on a 62GB host). Pin the heap to 1GB and the page cache to 512MB; the full integration suite passes with these bounds at -n 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nges The GraphQL registry caches a fully-built GraphQLSchemaManager (~40MB of graphene types, ~1s to build) per schema hash, but nothing removed the previous hash when the same branch's schema changed: purge_inactive only fires when a branch is deleted. Every schema update therefore retained a complete historical GraphQL schema in each API worker process until restart. Record the hash each branch currently serves (the pre-existing _branch_hash_activation_by_branch_name field was declared for this but never written) and, when a branch activates a new hash, evict the old one once no other branch is on it. Eviction also prunes registered types that no remaining schema references, using _reference_hash_schema_map as its docstring always intended, and purge_inactive now reuses the same path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…asses initialize_registry cleared the whole GraphQL registry before every test class, so each of the ~74 integration test classes rebuilt the identical pristine core-schema manager from scratch (~1s CPU and ~40MB of allocation churn per class). Registry entries are keyed by schema content hash and stale hashes are now evicted when a branch's schema moves on, so the blanket clear is no longer needed: the pristine manager is a cache hit for every class while mutated-schema leftovers still get dropped. Full backend/tests/integration suite passes with the cache retained. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The async neo4j driver and the query machinery produce reference cycles faster than the default gen-2 GC cadence collects them; profiling the integration suite showed workers carrying hundreds of MB of dead objects (~2M decoded driver values) between collections, on top of freed pages the allocator never returns to the OS. Worker RSS ratcheted to 1.9-2.7GB, which is what capped how many xdist workers fit on a host. Collect cycles and malloc_trim once per test class in the integration suite. Combined with the neo4j container memory bounds and the GraphQL registry eviction, the full suite now passes at -n 8 on a 62GB host (peak worker RSS 1.1-1.7GB) where it previously failed with OOM-driven timeouts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The integration suite is I/O-bound on the per-worker neo4j testcontainer, so it was leaving most of the huge-runner idle at the repo-wide default of 4 workers. With the neo4j container memory now bounded and per-class worker memory trimmed, 8 workers use less total memory than the previous 4 unbounded workers did, while roughly halving the job's wall time. Also cap MALLOC_ARENA_MAX so freed pages are returned to the OS under the higher worker count. Scoped to this job so the functional suite keeps the default worker count until its memory profile is validated separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fe2ac68 to
18a6cc7
Compare
Summary
API worker processes retain a full GraphQL schema (~40MB of graphene types, ~1s to build) for every schema version a branch has ever served: the registry caches managers by schema hash, but nothing evicts the old hash when a branch's schema changes —
purge_inactiveonly fires when a branch is deleted. Long-lived servers under schema churn grow until restart. Found while profiling the backend integration suites, where the same mechanism (plus driver garbage accumulation and host-sized neo4j auto-configuration) made memory the bottleneck that caps pytest-xdist parallelism.Key Changes
get_manager_for_branchnow records each branch's active hash (using the previously dead_branch_hash_activation_by_branch_namefield) and evicts a hash once no branch is on it, including registered types no remaining schema references — the cleanup_reference_hash_schema_map's docstring always described.purge_inactivereuses the same eviction path.Measurements (62GB/32-core host, full
backend/tests/integration)-n 4before-n 8before-n 8with this PR +MALLOC_ARENA_MAX=2In
test_generic_migrations(a schema-churn-heavy module) the registry previously accumulated one full GraphQL schema per schema update within the class (~1GB growth per class); with eviction it holds at most the active ones.Test Plan
backend/tests/unit/graphql/test_registry.py(hash reuse across calls and branches, activation-based eviction, protection while another branch still uses a hash, orphaned-type pruning,purge_inactivebehaviour).backend/tests/integrationsuite green locally at-n 4and-n 8with all changes.backend-tests-integrationandbackend-docker-integrationexercise the registry end to end.🤖 Generated with Claude Code
Summary by cubic
Fixes unbounded memory growth in API workers by evicting stale GraphQL schema caches when branches update, and reduces test worker RSS so we can run more xdist workers. CI now runs the integration suite with 8 workers faster and with lower memory.
Bug Fixes
purge_inactivereuses this and clears per-branch hash activation.Performance
Written for commit 18a6cc7. Summary will update on new commits.