Use special priority connection for config GET/HEAD requests

[tonistiigi]

After #2247 active TCP connections per host are limited to avoid things blowing up. While testing #2254 with a network limiter I spotted a case where one build was ongoing and doing a pull, other one couldn't really start because the image config resolve in Dockerfile was stuck in a queue blocking the whole build until the layers from the other build completed.

I wonder if it would make sense to have certain high-priority requests like HEAD and GET for manifest and config blobs that could get a single extra connection. These requests are small and don't take much time but they may unblock big parts of a build.

Not sure how this technically can be achieved with the current use of x/sync/semaphore package.

@sipsma @aaronlehmann

[aaronlehmann]

This seems like a good idea to me.

How about creating the semaphore with NewWeighted(2*connsPerHost+1), and using a weight of 2 for normal requests and 1 for "high-priority" requests? If we also care about limiting the number of total connections strictly to connsPerHost, we could also have a second semaphore were we use weight=1 for everything (similar to what's in place now). A request would have to acquire both before proceeding.

[tonistiigi]

with NewWeighted(2*connsPerHost+1), and using a weight of 2 for normal requests and 1 for "high-priority" requests

It doesn't look like the library supports this. Once weight 2 is blocking it seems to block everything. https://play.golang.org/p/_k5ygRP5lTm

I guess a helper method AcquireAny() could be made that calls Acquire in goroutines and high-priority one get extra semaphore.

[aaronlehmann]

How about this instead?

sema1 := semaphore.NewWeighted(connsPerHost)
sema2 := semaphore.NewWeighted(connsPerHost+1)
...
if !highPriority {
    sema1.Acquire(1)
    defer sema1.Release(1)
}
sema2.Acquire(1)
defer sema2.Release(1)

[sipsma]

+1 to Aaron's suggestion of two semaphores. You could probably make the other suggestion of 2*n+1 work if you use TryAcquire in the retry loop (rather than Acquire outside of it), but I think that would be more messy than just having two semaphores.