gaborgsomogyi commented on code in PR #28136:
URL: https://github.com/apache/flink/pull/28136#discussion_r3275153916


##########
flink-filesystems/flink-s3-fs-native/src/main/java/org/apache/flink/fs/s3native/S3ClientProvider.java:
##########
@@ -249,6 +261,27 @@ int getAssumeRoleSessionDurationSeconds() {
         return assumeRoleSessionDurationSeconds;
     }
 
+    @VisibleForTesting
+    boolean isUseCrt() {
+        return useCrt;
+    }
+
+    @VisibleForTesting
+    @Nullable
+    Double getCrtTargetThroughputGbps() {
+        return crtTargetThroughputGbps;
+    }
+
+    /**
+     * Exposed for tests to verify which async-client implementation (CRT vs 
Netty) was actually
+     * constructed. Not intended for production use; callers should go through 
{@link
+     * #getTransferManager()}.
+     */
+    @VisibleForTesting
+    S3AsyncClient getAsyncClient() {
+        return asyncClient;
+    }

Review Comment:
   `asyncClient` is never closed. `S3TransferManager.close()` only closes the 
underlying `S3AsyncClient` when it owns it (i.e. when built via 
`s3ClientBuilder()`) — if the client is provided externally via 
`.s3Client(asyncClient)`, the transfer manager sets `isDefaultS3AsyncClient = 
false` and skips closing it. Since `asyncClient` was introduced as a separate 
field in this PR, it is now leaked on every provider shutdown.
   
   Add an explicit close in `closeAsync()` alongside `s3Client`:
   ```java
   try {
       asyncClient.close();
   } catch (Exception e) {
       LOG.warn("Error closing S3 async client", e);
   }
   ```



-- 
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]

Reply via email to