On Sep 25, 2014, at 6:36 PM, weykent <weyk...@weasyl.com> wrote:
> On Sep 25, 2014, at 3:31 PM, Glyph Lefkowitz <gl...@twistedmatrix.com> wrote:
>
>> So, does anyone out there have any code which makes use of the
>> aforementioned bad attributes of ThreadPool, whose applications would break
>> if I removed them?
>
> Yes. Specifically, I am maintaining this AMP responder method:
Wow, cool.
So, okay, while I am a little unhappy that you're using this API in a slightly
unfortunate way, that feeling is eclipsed by the joy that The Process Works
:-). Thank you very much for responding so promptly, so specifically, and
including the exact code that we need to discuss.
> @FetchThreadPoolStats.responder
> def fetchThreadPoolStats(self):
> pool = self.factory.threadPool
> return dict(
> threadsWaiting=len(pool.waiters),
> threadsWorking=len(pool.working),
> workerQueueSize=pool.q.qsize(),
> )
>
> These statistics are chunked into graphite and displayed as a nice little
> graph. They’ve been quite useful on some occasions: sometimes all of the
> threads in a thread pool for a WSGI application got blocked, and the symptoms
> were that all of the requests coming in were timing out at the proxy in front
> of twisted. We were able to quickly tell that the problem was the WSGI thread
> pool because the threadsWorking count was at its limit and the
> workerQueueSize was skyrocketing.
Analytics, monitoring, logging, and statistics have historically been a blind
spot for Twisted and I am really glad you're bringing this up. I was thinking
about bringing it up in my original message, but as my colleagues and friends
will tell you, I tend to put too many words into emails, so unless I'm sure I
need those words, I will delete them.
The new interface should very definitely have metrics-gathering functionality,
and possibly even a logging component.
>> If so, how can I make you feel as bad about yourselves for using it as I
>> feel bad about myself for writing it? ;-)
>
> I don’t really see this as an abuse of the public interface. If possible, I’d
> like to see this diagnostic information preserved in the new interface, as I
> consider it invaluable information as long as we’re using twisted as a WSGI
> runner.
It's not exactly an "abuse", because the public interface is there, and you
have a use-case, and you're satisfying it with what's available.
For the purposes that you are using these attributes, I actually don't have a
problem reproducing them at all. It should be pretty straightforward. My
concern with exposing them overall is that what you've got with "ThreadPool.q"
is not a queue that has a qsize() method that you can inspect, it's that it's a
Queue that you can put things into, get things out of, and generally muck about
with.
Similarly pool.waiters is not simply an object with a __len__ that tells you
how active the threads are, it's a list of Thread objects which you could
potentially change the names of, turn into daemon threads, join(), and so on.
More importantly it's a list that you could remove Thread objects from and that
would affect their interactions with the pool.
How about this: would you mind if ThreadPool were modified to still populate
these attributes for monitoring and debugging purposes, but completely stopped
honoring any destructive operations on them? In the case of "pool.q", there is
no single Queue responsible for the whole pool, so I would like to make "put",
"get", and "join" actually raise exceptions; in the case of "waiters" and
"working" I guess I could fish out some actual Thread objects, that just
involves a little bit of sad abstraction violation internal to the
twisted.threads.Team implementation.
My inclination would be to expand the thread pool to accommodate these usages,
but still deprecate these attributes in the next version; but add a better
"statistics" method that returned an object with "active", "pending", and
"idle" attributes (all integers).
The most important thing that I want to make sure nobody wants is to keep
overriding (or calling) startAWorker, stopAWorker, __getstate__, __setstate__.
Supporting callers of threadFactory and currentThread would be easy, but the
attributes aren't really there for calling, they're there to be stubbed, and
stubbing them won't keep working without tons of extra work.
So would that work for you?
> I will say that it is certainly possible for me to emulate these attributes,
> but that would require touching implementation details of WSGIResource. I’m
> not sure which is worse.
Touching private (underscore-prefixed) implementation details is explicitly not
supported - if you did it that way, you're on your own :-). And, as the
end-user in this discussion, you win the argument by default. (You can see how
I feel about supporting users here:
<https://twitter.com/mjg59/status/514670470260465664>) Plus, if we're going to
work on the thread-pool and have newer, better interfaces, WSGIResource's
implementation is likely to change.
Thanks a bunch for helping us improve Twisted,
-glyph
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
