Admaing commented on PR #18612: URL: https://github.com/apache/dolphinscheduler/pull/18612#issuecomment-5630958552
> ** Separate connection pools for enabled and disabled TCP keepalive** > > `OkHttpUtils.java:315–320` derives both client variants from `CLIENT.newBuilder()`, which shares the original connection pool. In OkHttp 4.12.0, connection eligibility uses `Address.equalsNonHost()`, which does not compare the plain `socketFactory`. > > If a request with keepalive disabled leaves a reusable connection in the pool, a subsequent keepalive-enabled request to the same origin can reuse that socket without invoking `KEEP_ALIVE_SOCKET_FACTORY`. TCP keepalive remains disabled, so the new option does not reliably address the idle-connection failure. The reverse also occurs: disabled requests can reuse enabled sockets. > > Please maintain separate reusable pools for the two settings. Add a regression test that sends consecutive requests to the same origin with different settings and verifies the actual socket keepalive state; successful responses alone do not verify this behavior. Confirmed — Address.equalsNonHost$okhttp ignores socketFactory, so the shared pool made the option unreliable in both directions. KEEP_ALIVE_CLIENT is now built from `new OkHttpClient().newBuilder()` so it has its own ConnectionPool, and getHttpClient(...) picks the base client by the flag. OkHttpUtilsTest sends consecutive requests to the same origin with different settings, and asserts the other setting's pooled connection count stays 0, that the keepalive client's socket factory produces a socket with getKeepAlive() == true, and (recorded on the test server) that the two requests did not share one TCP connection. It fails if I revert to a shared pool. getHttpClient(...) is now package-private + @VisibleForTesting so the test stays on public OkHttp APIs instead of reflecting into okhttp3.internal.*. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
