Pulp

Is it worthwhile to expose connection_limit_per_host to users? It seems hard to explain the two different fields in terms of a Pulp user's perspective and I am not sure it's very useful to set connection_limit_per_host.

bmbouter wrote:

The ArtifactDownloader has a max_downloads restrictor. This is an identical feature to the connection limiting offered with aiohttp itself. Specifically aiohttp offers two features with its TCPConnector:

Nitpick: the feature is not identical. Multiple stages may use the same remote downloader factory (e.g. one doing downloads, one doing API requests). But it is probably not worth the additional complexity to limit downloads per stage and connections globally via the connection pool.

gmbnomis wrote:

Nitpick: the feature is not identical. Multiple stages may use the same remote downloader factory (e.g. one doing downloads, one doing API requests). But it is probably not worth the additional complexity to limit downloads per stage and globally via the connection pool.

Now that you mention it, I can see what you are saying. I also agree it's probably not worth to have it per-stage and globally. I think it would also be confusing to end-users.

I just realized that there is probably a huge difference between the previous implementation limiting the active downloaders and the new one relying on aiohttp's connection limit. I have not tested it, but I think that the following may happen:

If a downloader gets back one of the HTTP codes which trigger a retry, the exponential backoff kicks in https://github.com/pulp/pulp/blob/60700bc51a2f5d49030950129edc9c723333e82d/plugin/pulpcore/plugin/download/http.py#L161. This means that the downloader will free up a connection and sleep. As we may have > 200 downloaders waiting for a connection, a new downloader will hit the server immediately without a backoff. If the server sends the 429 (or other) return code because we hit a limit, it will continue to do so. Even if there is only one artifact to download per content unit, Pulp will hammer the server with 200 requests until all of them are retrying in the worst case. And 200 downloaders in retry will probably still cause lots of load.

We could reduce max_concurrent_content to e.g. 10 to ameliorate this effect, but then, the entire stage blocks when there are 10 content units scheduled for download. The main point about max_concurrent_content is that the stage allows declarative content not needing downloads to pass it on a "fast lane" until we have such a large number of content units that we don't want to buffer them anymore. It is not intended to limit the number of concurrent active downloaders (this is exactly what the Semaphore that is gone now was for). Furthermore, even 10 content units may need 20 downloads each and we have the same effect as before.

Thus, if the scenario outlined above is valid and if we want to limit the number of active downloaders to achieve proper backoff, I am afraid that the simplest solution is to revert to the previous implementation of the stage and use the new configuration in the remote to limit the number of downloaders (and possibly, the number of connections at the same time).

I verified that the retry behavior is as expected, see issue https://pulp.plan.io/issues/4075