282441 - map tiles sometimes fail to load when pipelining is enabled

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Darin Fisher]

map tiles sometimes fail to load when pipelining is enabled

after some analysis the problem appears to be caused by the fact that the server
closes the connection after serving only the first request in a pipeline.  as a
result, we end up retrying requests a lot.  eventually some of the requests
reach their maximal retry limit.

this problem is only really noticed when a lot of tiles need to be fetched.

the solution is probably to remember sites that fail in this manner, and then we
should avoid pipelining to them.  it might be good to make this be a simple LRU
based cache of recently rejected servers (use nsHttpConnectionInfo::HashKey).

Comment 1

[Darin Fisher]

my analysis of this problem was actually wrong.  in fact, it turns out that the
src attribute of image elements are changed so frequently, that we are canceling
http loads left and right.  as a result, we end up breaking pipelines a lot, and
that is what causes the problem.  though i did see some connection resets,
client driven cancelation was in fact the dominating factor.

Comment 2

[Darin Fisher]

Attachment: v1 patch

This is an experimental patch.	With this problem, we disable pipelining for a
transaction whenever it is being restarted.  This makes it so that we adapt to
frequent restart problems induced by pipelining-related connection failures. 
We can still do better than this by avoiding the connection close when a
pipelined request is canceled, but for now I think this will go a long way to
helping.

Comment 3

[Darin Fisher]

With some testing, I'm convinced that this is the right patch.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 176546 [details] [diff] [review]
v1 patch

Seems reasonable...

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 176546 [details] [diff] [review]
v1 patch

will this fix some of the other pipelining bug reports we have?

Comment 6

[Darin Fisher]

> will this fix some of the other pipelining bug reports we have?

yes, this may in fact be a duplicate of some other bug reports, but i don't have
the cycles to go through those right now.

Comment 7

[Darin Fisher]

fixed-on-trunk

Comment 8

[Robert Kaiser]

It seems that this checkin is to blame for brad malloc increasing from 286K to
4.96M, and leaks increasing as well.

Comment 9

[Darin Fisher]

> It seems that this checkin is to blame for brad malloc increasing from 286K to
> 4.96M, and leaks increasing as well.

nope, it was a fluke in the brad tinderbox apparently.