Apache client connection pool instrumentation #498
Conversation
| Function<PoolingHttpClientConnectionManager, T> extractor, BiFunction<T, T, T> combiner, T initial) { | ||
| T current = initial; | ||
| Iterator<WeakReference<PoolingHttpClientConnectionManager>> iterator = pools.iterator(); | ||
| while (iterator.hasNext()) { |
There was a problem hiding this comment.
For loop doesn't allow the iterator.remove operation we use when a garbage collected item is encountered.
| // Increased from two seconds to twenty-five seconds because we have strong support for retries | ||
| // and can optimistically avoid expensive connection checks. | ||
| connectionManager.setValidateAfterInactivity( | ||
| Ints.checkedCast(Duration.ofSeconds(25).toMillis())); |
There was a problem hiding this comment.
I snuck in a behavior change here, I can pull this into a separate PR if you prefer. I dropped it here because this PR adds direct access to the PoolingHttpClientConnectionManager, which is the only to configure setValidateAfterInactivity
| .evictIdleConnections(55, TimeUnit.SECONDS) | ||
| .setMaxConnPerRoute(Integer.MAX_VALUE) | ||
| .setMaxConnTotal(Integer.MAX_VALUE) | ||
| // TODO(ckozak): proxy credentials |
There was a problem hiding this comment.
removed this stale TODO, it's TODONE
| javaPackage: com.palantir.dialogue.hc4 | ||
| javaVisibility: packagePrivate | ||
| namespaces: | ||
| dialogue.client.pool: |
There was a problem hiding this comment.
The metric name doesn't reference the apache client, I tried to keep it as general as possible so we could implement it for other clients in the future if applicable. Thoughts?
There was a problem hiding this comment.
I like the idea of having consistent metric regardless of underlying client implementation. We can update the docs if we decide to work on another client
|
👍 |
|
Released 0.16.2 |
See #457
After this PR
==COMMIT_MSG==
Apache client connection pool instrumentation
==COMMIT_MSG==