Increase maximum number of allowed connections in HTTP connection pool #424
Conversation
Not all requests release connections, which leads to connection pool starvation and lock-ups during files upload. Apache's HTTP client handles "lost" connections by a GC hook and removes unreachable objects from the pool. However, this mechanism requires actual GC to occur, which may not happen if the app is otherwise idle. One way of observing this problem is to: 1) start upload of ~30 decently sized photos (few mbytes) 2) hang round in Uploads screen and observe progress bars 3) stop the app using debugger and observe FileUploaderThread being blocked in MultiThreadedHttpConnectionManager.doGetConnection() (calling wait()) 4) when upload stalls, force GC using Android Studio profiler; this should unblock the connection pool and resume uploads Increasing maximum number of allowed connections gives more room for GC to kick in and reclaim stale connections, which in turn should greatly improve stability of multi-file uploads. Other method would be to walk around the app, causing memory allocations and triggering a GC as a side effect. Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com>
4ab104d to
78e592b
Compare
|
@tobiasKaminsky @AndyScherzinger I believe we have some user reports of stale uploads... this is nasty workaround that should improve the situation. Proper solution would require tracking down all leaks and eliminating them. HTTP method has explicit API to release the connection, so this should not be a massively difficult task. In the meantime, this is a cheap work-around that seems to work. While benchmarking it, my connection pool accumulated 10 connections max, just to drop to 2 when GC kicks in. Your mileage may vary. |
I cannot see how this is caused by me. :) Flaky test? |
Lint
SpotBugs (new)
SpotBugs (master)
|
|
Possibly a solution for nextcloud/android#5758 and nextcloud/android#3715 |
|
Nice trick!
We should do this (and I thought we are doing this, but better check it)… |
Lint
SpotBugs (new)
SpotBugs (master)
|
|
On Tue, Apr 14, 2020 at 08:25:51AM -0700, Tobias Kaminsky wrote:
We should do this (and I thought we are doing this, but better check it)…
It might be tricker than you think, as the HTTP client uses some
heuristics in HttpMethodDriver to release the connections when it
thinks it's no longer needed. Apparently we have some edge cases.
I'm not sure if the manual release will not clash - I haven't
dive into the API so deeply to fully understand how it works.
It's design doesn't encourage me to invest more time into it...
|
Also we want to switch to okhttp, so it would be somehow a waste of time ;-) |
Not all requests release connections, which leads to connection
pool starvation and lock-ups during files upload.
Apache's HTTP client handles "lost" connections by a GC
hook and removes unreachable objects from the pool.
However, this mechanism requires actual GC to occur, which
may not happen if the app is otherwise idle.
One way of observing this problem is to:
being blocked in MultiThreadedHttpConnectionManager.doGetConnection()
(calling wait())
should unblock the connection pool and resume uploads
Increasing maximum number of allowed connections gives
more room for GC to kick in and reclaim stale connections,
which in turn should greatly improve stability of multi-file
uploads.
Other method would be to walk around the app, causing memory
allocations and triggering a GC as a side effect.
Signed-off-by: Chris Narkiewicz hello@ezaquarii.com