connection_limit parameter ignored after request is called for the first time

[agoodm]

After the first time HTTP.request gets called, the connection_limit parameter is ignored and the number of open connections is set to whatever was used from the first call. I verified this behavior via lsof -c julia.

See: https://discourse.julialang.org/t/http-jl-async-is-slow-compared-to-python-aiohttp/96736/38?u=agoodm

[mkitti]

The issue occurs here. After a pod is created and stored in pools.pod, it does not attempt to verify that the retrieved pod has the desired characteristics. This is probably an API issue more than anything.

HTTP.jl/src/connectionpools.jl

Lines 252 to 257 in 2e8c99d

	function acquire(pool::Pool{C}, key, conn::C; kw...) where {C}
	pod = lock(pool.lock) do
	get!(() -> Pod(C; kw...), pool.pods, key)
	end
	return acquire(pod, conn)
	end

[mkitti]

My thought here is that we should display a warning when keywords are being ignored.

 function acquire(pool::Pool{C}, key, conn::C; warn_unused_keywords=true, kw...) where {C} 
     pod = lock(pool.lock) do
         if warn_unused_keywords && !isempty(kw) && haskey(pool.pods, key)
             @warn "The following keywords were not used since the connection pool pod already exists:" kw
         end
         get!(() -> Pod(C; kw...), pool.pods, key) 
     end 
     return acquire(pod, conn) 
 end 

[quinnj]

Yeah, this is indeed a bit of an awkward API issue; on the one hand, it's not desirable that whatever the connection_limit is set to initially is permanently set for all subsequent requests and so different connection_limit values are then subsequently ignored.

On the other hand, it's also awkward if we make connection_limit part of the "key" for the pod; that would mean that there's essentially a separate pod per connection_limit value and to "bypass" a connection limit, you just need to provide a new, unique connection_limit that hasn't been seen before.

I can currently think of 2 possible ways forward:

• We only make connection_limit a global parameter, so you'd have to call HTTP.set_connection_limit. The tricky part here is that what if you want to set different connection limits for different hosts/ports? Should we allow you to specify the current full "key" of connection pool pods if desired?
• Or we keep connection_limit on requests, but just update a pod's connection limit if a different value is passed? I don't love this option because it makes a pod's connection limit non-const and it seems like you could get into weird scenarios, like if you initially pass connection_limit=10, but then pass connection_limit=1, but there are still 9 connections out there running?

[quinnj]

Though thinking further, I guess you still have the same awkwardness with the global setting: I set HTTP.set_connection_limit(10), then make a request and the pod is created w/ a limit of 10, then call HTTP.set_connection_limit(1), but subsequent requests through the same pod keep the limit of 10.

[mkitti]

The public API appears to be fully functional. In reality, there is a hidden persistent state.

The circumstances where this was discovered between sessions, groupings of requests.

We could either expose the hidden persistent state to the public API. Alternatively, we make the state less persistent, possibly via a public API.

A public API for this might expose a HTTPContext containing all of the persistent state with the default context being a global context.

A simple implementation might be to allow one to set a new global context so that the library can return to its initial state for new connections. Existing connections can continue to use the prior context in which they were created.

A more advanced implementation would allow the user to manage multiple contexts directly.