Possible thread leak #343
Comments
chrisrossi
added
🚨
|
This happens on my computer locally - I can see the thread count go up in a python process with each call to We also use app engine standard37 and flexible python37. |
|
I'm trying to reproduce this and haven't so far. Here's a sample app I'm using to test: Here is a transcript of a session: The threads remain constant, despite the call to One difference is my sample app creates the client only once and encapsulates the context in a middleware. I wonder if you still see this issue using the middleware in my sample app rather than instantiating a client in a Flask view? Also, you can try upgrading to NDB 1.0.1. |
|
Thanks for the suggestion @chrisrossi! There is not a lot of good examples for how to properly make a middleware for webapp2. I'm trying to fit it into my sprint - I will let you know how it goes when we have time. |
|
Here is your example adapted to include the middleware: I've done some testing with this and haven't found a thread leak. I'll do a little more stress testing before putting this away, though. |
|
Ok, I just realized the cache was masking the issue. I have been able to reproduce the thread leak. Since NDB doesn't create threads, it's obviously an issue with something we're using. It's unclear at this time if there will be a solution on the NDB side, or if it will wind up being something for the maintainers of a library we depend on. |
|
I've also confirmed that the leak occurs with webapp2 but not Flask, which makes it more likely to be related to/the same as #336. |
|
That's very interesting!! I wonder if this category of problems is an issue with webapp2 specifically and not GAE Py2 in general. If so it would be much easier to fix. Time permitting, let's try to verify that #336 is reproducible on webapp2 but not on flask, and, especially, if both this and #336 are reproducible locally, outside of GAE Py2. |
|
Thanks for the extra effort! I just verified your results; We still see the thread leak with the middleware as well. More context: over the years we have seen memory leaks and thread leaks. Like variables not being destroyed after the handler finishes. Which leads to critical OOM errors on GAE. My hunch is something to do with context managers. Like |
|
I can verify that we're leaking Anyway. Still investigating. This is a tricky one. |
This brings our practice in line with `google.cloud.datastore`, which also creates one channel per client. This works around a resource leak issue by not requiring the channel to clean up after itself properly in normal usage. The root cause of that issue seems to lie somewhere in `google.auth`, which is where I will follow up. Fixes googleapis#343
|
#362 works around this issue. You will still have to limit yourself to one |
|
Thanks @chrisrossi and team. I verified the resource leak is gone after using python-ndb:master and the middleware as demonstrated here Nice work! It would be nice to be able to follow the progress in |
This brings our practice in line with `google.cloud.datastore`, which also creates one channel per client. This works around a resource leak issue by not requiring the channel to clean up after itself properly in normal usage. The root cause of that issue seems to lie somewhere in `google.auth`, which is where I will follow up. Fixes #343
|
I'm in App Engine Standard on Python 2.7, and I'm still seeing the issue after installing Has someone tried @chrisrossi 's fix on Standard 2.7? And any ideas for a workaround? |
|
Hi @waltermoreira I did some testing just now with Standard 2.7 and I'm not seeing a thread leak. Is it possible you're getting NDB from PyPi and not GitHub? I don't think this particular fix has seen release yet. |
|
I'll go ahead and cut a release now so that that isn't an issue going forward. |
|
Thanks for the response @chrisrossi and @andrewsg . I did make sure to install from github, and I checked I'm actually importing the new code by adding logging to it. I could be doing the instantiation of the client wrong, though. I know it might be out of scope, @chrisrossi , but would you mind sharing if possible your experiment? |
|
Sure, here is a test app: Here it is in action: |
|
@chrisrossi Thank you so much for the example. This is weird. There must be something wrong with our environment. I copied and pasted your code and I'm still getting a hanging thread at the end of the request to From your experience with appengine, do you have a suggestion to where to concentrate our debugging? |
|
The issue of threads staying open beyond the end of an App Engine request is actually issue #336. I thought they might be related, but it looks like I need to turn my attention to that one. The reporters suggest a workaround of calling |
Hi,
We investigated a bug about a process running out of threads in a long lasting process with thousands of requests being made. After researching we narrowed down the cause to
key.get; we also can reproduce withquery.fetch.The below functions are ran in threading.py
startwhenkey.getis called. It seems like one or more of them may have a thread that needs to close at the end of the request?Thank you for the help!
Environment details
Steps to reproduce
key.get()Code example
When making the request, watch the number of threads in the python process. You should see it climb for each request. In the screenshot below it is the highlighted process with 19 threads.
Stack Trace
possibly related to #336
The text was updated successfully, but these errors were encountered: