fixing errors related to python upgrade - #29
Open
fcollman wants to merge 8 commits into
Open
Conversation
app/__init__.py imported ..ingest.cli at module scope purely to register Flask CLI commands. That import chain reaches pcgl2cache.core.features, which pulls in sklearn (43 MB), sklearn.decomposition (10 MB) and scipy.ndimage (22 MB). The registration itself was already conditional on USE_REDIS_JOBS, and the API deployment runs DevelopmentConfig where that is False -- so every uwsgi worker paid for imports whose functions were never called. core.features computes L2 features at ingest time (PCA, EDT); the API only reads precomputed values out of Bigtable. Moving the two imports into the branch that uses them, measured in a deployment pod with a fresh interpreter: before: 8 MB -> 216 MB after create_app() sklearn+scipy.ndimage loaded after: 8 MB -> 140 MB after create_app() neither loaded 76 MB per process, 35% less. Route registration is unchanged (8 routes, including the attributes endpoint), and both deferred imports still resolve for the USE_REDIS_JOBS=True configurations that actually register the commands. Context: the API pods run 8 uwsgi workers minimum against a 900Mi memory request, putting their floor at ~1.05 GiB before serving anything, which was driving repeated node-pressure evictions on a shared node pool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…per request get_l2cache_client built a fresh BigTableClient -- and with it a new gRPC channel -- on every request, and get_l2cache_cv built a fresh CloudVolume, re-parsing the info document each time. Both were called from the attributes endpoint, so a worker did this once per request. The channels were dropped immediately, but the allocation churn sets the worker's heap high-water and glibc does not return it. Measured on minniev7: workers sit at ~176 MB anonymous against the uwsgi master's 157 MB despite the master having *more* loaded, and a worker reaches roughly 68 MB above its post-import baseline within about 20 requests, then stays flat (25 req -> 208 MB, 71 req -> 212 MB). That flatness is why this looked like a fixed cost earlier: the high-water is reached almost at once, not that the churn is free. It also explains why making the ingest imports lazy freed 76 MB of imports without changing RSS -- the churn simply expanded into the space that freed up. Long-lived clients are the intended pattern: gRPC reconnects internally, and the CloudVolume is only read for metadata here (resolution, bounds, graph_chunk_size, meta), never voxel data, so one instance is safe to share. Caches are keyed on the backing l2cache_id / cv_path rather than graph_id, so two graphs pointing at the same table share one client and the cache stays correct if config is rebuilt. The per-request config lookup and its assert are unchanged. Also drops the module-level CACHE dict, which was declared and never referenced anywhere in the package; these caches are evidently what it was meant to be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
this was referencing several options which are out of date when you build the image with the current constraints on python 3.13. This whole repo needs to shift to a more reproducible framework for building the docker image, ideally off of a uv.lock file to intentionally trigger library upgrades, but these are good underlying code fixes to enable more recent flask versions.