-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bug] Too many open files
error
#226
Comments
This seems to be the offending logic:
|
I genuinely do not know if this will resolve the issue. I added an lru_cache to get_config. I clear that cache when the app config is updated. I also clear that cache when we run the context managed get config.... What this means is that all API routes will pull cached data, I think, using the get_config_depends function, whereas the UI routes will pull direct from the file. Still not perfect and very likely to be error prone. Will continue to monitor and make changes as needed. But, closing the issue for now. |
Now, it's a different file:
|
Potentially linked to:
If so, consider upgrading to the most recent version of uvicorn (0.30.1). |
Even with caching, when we make a single change to the app config in production from the UI, it logs 21 (!) changes within a second of each other... This has got to stem at least partially from the number of uvicorn workers (3) ... but still, it is inexplicable why there are so many file handles opened by this.... Notably, when we make a config change (admittedly, for a different config) from the Restful API, we get far fewer changes reported (consistently 12): I will note that the UI-based config changes report more variable numbers of config reloading. This might be due to a mix of factors, including the javascript API calls made from the UI. |
This may be tied to #221. In it, we made the document database a dependency injection for each route. The benefit is that we can now edit the app config at runtime (eg. through the UI) without needing to restart the app. However, the issue is that a logger object is instantiated by the document database each time the document database connector is instantiated. That was written before the application moved to the document-database-as-dependency-injection setup. So, if we are playing a game of whack-a-mole, we should modify the ManageDocumentDatabase class to expect a logger as a param, not to instantiate one. |
[bug] production_uvicorn_snippet.log I struggle to understand what a solution to this looks like. Perhaps: a nightly cron job to restart the server ... but this works against uptime/availability requirements in most production scenarios. Is there an architectural issue here that we have not been thinking through? Are there ways to cache more files and reduce the number of open file handles? |
References:
The text was updated successfully, but these errors were encountered: