Add java docs to explain how the max connection settings are used #33
Conversation
There was a problem hiding this comment.
It seems reasonable to me, but I don't know much about this.
Tagging @jhaber + beaast (@kmclarnon @Xcelled @snommit-mit)
| public int getMaxConnections() { | ||
| return maxConnections; | ||
| } | ||
|
|
||
| /** | ||
| * Unlike maxConnections, the HttpClient will use this per-host setting to IMMEDIATELY FAIL any requests |
There was a problem hiding this comment.
nit: should these variable and class references be javadoc links, eg {@link #getMaxConnections} or w/e? That way people can easily browse the code from this comment.
There was a problem hiding this comment.
oh second thought, would it be ok to merge without it? I haven't been able to get back to this (I got locked out of public GH for a month between writing these comments and opening the PR), and this is at least better than having no comments
| * instead, a {@link org.asynchttpclient.shaded.exception.TooManyConnectionsPerHostException} is thrown from the client immediately. | ||
| * | ||
| * Note, {@link #getConnectTimeoutMillis()} doesn't come into play in this connection limiting. | ||
| * Instead, the internal `acquireTimeout` is the timeout for acquiring a connection permit; |
There was a problem hiding this comment.
Should this be connect timeout instead?
There was a problem hiding this comment.
No, I think what I meant to explain here is that the connect timeout can only come into play after the client has acquired a connection permit, so it's the acquireTimeout defaulting to 0 that causes "The client does NOT block / wait for any amount of time if it gets a request when the max connections per host are already used up" - and since acquireTimeout isn't exposed there's no way to have it not throw immediately when maxConnectionsPerHost is hit. The connect timeout should be for establishing a connection to the API after a connection permit is acquired (technically it could influence connection limiting if it was set way too high, but it isn't part of the immediate failure behavior).
Does that make sense? I could try to reword it if that might help
I could provide code links for these things if you'd rather not just take my word for it, though I think at least @jschlather has looked into this behavior as well and could potentially vouch
@stevegutz @pH14