useSystemProperties breaks custom connection pooling
In this builder chain, the call to useSystemProperties() effectively results in ignoring the values set by setMaxConnTotal(maxConnections) and setMaxConnPerRoute(maxConnections). And this isn't order-dependent, because the actual properties used by the underlying client are determined during the call to build with this code in the upstream Apache client
So I see basically 3 choices:
-
Stop calling
useSystemProperties(). This will require figuring out how to go back and add proxy support that motivateduseSystemProperties. -
Explicitly call
System.setProperty("http.keepAlive", "true")andSystem.setProperty("http.maxConnections", maxConnections)before building the client. (Somebody with a bit more java mastery wanna weigh in on the side affects of setting properties like this? Any risks to creating multipleAlgorithmiClientinstances with different connection pooling?). This also means that we have to accept thatmaxConnTotalis always 2x themaxConnPerRoute. -
PR to try and convince upstream to replace that block with one where it only uses the system properties for those values if they weren't explicitly set. (I presume we could use the 2nd option in the meantime of waiting for such a PR to land.)
Wanna weigh in @kennydaniel, since I believe you green-lighted the useSystemProperties change?
Hmm, that is tricky. Calling System.setProperty could have sideeffects if someone includes our client in their code base, but have OTHER http clients that have useSystemProperties() enabled. In my opinion this makes option 2 not ideal. From reading the apache source code, I don't see an easy way to work around this.
I think option 1 seems like the least bad option. I grepped the apache source code for System.getProperty and it appear there are only 4 proxy properties used here: https://github.com/apache/httpcomponents-client/blob/22902593e78d56423709b66abf4757082ddced50/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/SystemDefaultCredentialsProvider.java#L141