Last Comment Bug 767512 - Fix connection pressure for non-SSL CONNECT
: Fix connection pressure for non-SSL CONNECT
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla16
Assigned To: Nobody; OK to take it and work on it
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on: 713026
Blocks: 767516
  Show dependency treegraph
 
Reported: 2012-06-22 12:01 PDT by Jason Duell [:jduell] (needinfo me)
Modified: 2012-07-10 15:50 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1: fix IsUnderPressure for non-SSL CONNECT (1.92 KB, patch)
2012-06-22 12:01 PDT, Jason Duell [:jduell] (needinfo me)
mcmanus: review+
Details | Diff | Splinter Review

Description Jason Duell [:jduell] (needinfo me) 2012-06-22 12:01:40 PDT
Created attachment 635851 [details] [diff] [review]
v1: fix IsUnderPressure for non-SSL CONNECT

Patrick: let me know if you think this merits beta/aurora. W/o this patch CONNECT connections will count as regular ones, so regular cxns will report "underpressure" more than they should.  Seems like a perf hit but not full stoppage of connectivity. (We already fixed things so non-SSL CONNECTS don't count toward the per-proxy connection limit:  we just didn't fix it for IsUnderPressure() yet).
Comment 1 Jason Duell [:jduell] (needinfo me) 2012-06-22 12:15:00 PDT
Since this depends on bug 713026 and that isn't making it to aurora/beta, it's moot to consider backporting this as well.
Comment 2 Patrick McManus [:mcmanus] 2012-06-25 06:26:47 PDT
Comment on attachment 635851 [details] [diff] [review]
v1: fix IsUnderPressure for non-SSL CONNECT

Review of attachment 635851 [details] [diff] [review]:
-----------------------------------------------------------------

this won't have any meaningful impact as "under pressure" only applies to pipelining, and websockets connections don't pipeline (and they are quickly upgraded out of the connection count completely). But its right nonetheless :)
Comment 3 Jason Duell [:jduell] (needinfo me) 2012-06-25 12:49:53 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ff98d3e4d8d
Comment 4 Ed Morley [:emorley] 2012-06-25 13:41:48 PDT
Push backed out for M5 orange:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d45ca69b67a3

Couldn't see you on #developers or see any try run URLs listed in-bug in the four bugs in the push, or I would have tried to find out if just part of it could have been backed out.

https://hg.mozilla.org/integration/mozilla-inbound/rev/51cff2123f45
Comment 5 Jason Duell [:jduell] (needinfo me) 2012-07-09 13:00:44 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/90b265723879
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-07-10 15:50:12 PDT
https://hg.mozilla.org/mozilla-central/rev/90b265723879

Note You need to log in before you can comment on or make changes to this bug.