Opened 18 years ago
Last modified 4 years ago
#601 defect new
ThrottlingFactory doesn't throttle static web resource
Reported by: | mg | Owned by: | JohnDoeee |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | web | Keywords: | |
Cc: | acapnotic, itamarst, jknight, mg | Branch: | |
Author: |
Description
Attachments (1)
Change History (10)
comment:2 Changed 17 years ago by
Investigating. The addition of __getattr__ on ProtocolWrapper <a href="http://svn.twistedmatrix.com/cvs/trunk/twisted/protocols/policies.py?r1=9765&r2=9767&p1=trunk/twisted/protocols/policies.py&p2=trunk/twisted/protocols/policies.py">in rev 9767</a> is the cause for the traceback -- ThrottlingProtocol and its transport end up having a namespace collision around "producer." haven't decided if this is also the reason for the failure to limit. Unfortunately the ThrottlingTestCase tests are marked as "skip."
comment:3 Changed 17 years ago by
<keturn> brilliant! ThrottleStuff doesn't work on web.static because web.static..pauseProducing is defined as "pass" <foom> keturn: as it should be. <foom> keturn: static.File is a non-streaming-producer, which means resumeProducing should be called once every time data is wanted so policies.ThrottlingProtocol doesn't support non-streaming producers. It ignores the "streaming" argument to registerProducer and doesn't change its behaviour accordingly. so either fix, or have it raise exceptions when it gets non-streaming producers registered to it.
comment:5 Changed 11 years ago by
Owner: | acapnotic deleted |
---|
Changed 10 years ago by
Attachment: | ticket-601-ratelimit-non-streaming-producer.patch added |
---|
comment:6 Changed 10 years ago by
Keywords: | review added |
---|
I have added a patch that'll fix the problem, but as I am uncertain this is a good way to solve it, I would like to request a preliminary review of my method of solving the problem.
A big problem with the ThrottlingFactory is that any producer can run rampant for up to a second before it is stopped. IF the producer is done and the ThrottlingFactory attempts to pause, it runs into a problem where self.producer is None (I am not sure why that happens).
As noted elsewhere, the way the ThrottlingFactory limits the traffic is bad, but with this patch only the way to do the actual throttling needs a makeover.
What I want to know is if this is an acceptable way to do the actual limiting.
comment:7 Changed 10 years ago by
Keywords: | review removed |
---|---|
Owner: | set to JohnDoeee |
- I am skeptical that this needs to be solved by changing twisted.internet.abstract. If the problem is that "policies.ThrottlingProtocol doesn't support non-streaming producers", the solution would be to add that support.
twisted.protocols.tls._PullToPush
and related code is a way to do that. - In the unlikely case that it does, lack of parenthesis means I have to remember precedence rules for "and" and "or".
- No tests.
comment:8 Changed 10 years ago by
Hm. That review seems rather snippy. My apologies, I guess I was having a bad idea. I appreciate someone trying to fix this! Like I said, though, using something like the _PullToPush
class is probably the way to go.
comment:9 Changed 4 years ago by
This may have been fixed, as HTTPChannel now internally adapts pull producers to push producers.