Fix connection leak in Prometheus and in Oauth2 tests #17411
Conversation
- avoid resource allocation in constructor - close okhttp client - close Response (fix connection leak) - do not use ImmutableMap as return type
okhttp's Response needs to be closed.
Also improve message in TaskId constructor checks.
findepi
added
the
no-release-notes
| @@ -140,11 +137,20 @@ protected abstract TestingHydraIdentityProvider getHydraIdp() | |||
| public void tearDown() | |||
| throws Exception | |||
| { | |||
| Logging logging = Logging.initialize(); | |||
There was a problem hiding this comment.
define "this".
logging reconfiguration was pre-existing here. i am fine removing it, if we tell me it's not needed.
| server, | ||
| hydraIdP, | ||
| () -> { | ||
| httpClient.dispatcher().executorService().shutdown(); |
There was a problem hiding this comment.
Should we have some kind of wrapper on OkHttpClient to make it closeable. It looks like it is not trivial thing to close it and an easy mistake to have.
There was a problem hiding this comment.
good idea. i will not do this within this PR though, hope that's ok
| @@ -164,18 +164,15 @@ private Map<String, Object> fetchMetrics(JsonCodec<Map<String, Object>> metricsC | |||
| public byte[] fetchUri(URI uri) | |||
| { | |||
| Request.Builder requestBuilder = new Request.Builder().url(uri.toString()); | |||
| Response response; | |||
There was a problem hiding this comment.
How did you found it? Can we have static code analyzer for it?
There was a problem hiding this comment.
okhttp reports leaked connection by logging a warning. I noticed "leaked" in the test output when trying to understand what's going on in #16933
it just happened that TestMarkDistinctOperator was running concurrently with TestOAuth2WebUiAuthenticationFilterWithJwt
i don't know about a way to automate that and refuse to do this as part of this PR, i hope this is ok
No description provided.