danielcweeks commented on code in PR #12799:
URL: https://github.com/apache/iceberg/pull/12799#discussion_r2070700873


##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java:
##########
@@ -384,30 +386,85 @@ public void deletePrefix(String prefix) {
     deleteFiles(() -> 
Streams.stream(listPrefix(prefix)).map(FileInfo::location).iterator());
   }
 
+  /**
+   * Returns the {@link S3Client} that is configured for this {@link 
org.apache.iceberg.io.FileIO}
+   * instance.
+   *
+   * @deprecated since 1.10.0, will be removed in 1.11.0; use {@link
+   *     S3FileIO#clientForStoragePath(String)} instead.
+   */
+  @Deprecated
   public S3Client client() {
-    if (client == null) {
-      synchronized (this) {
-        if (client == null) {
-          client = s3.get();
-        }
+    return clientForStoragePath("s3").s3();
+  }
+
+  /**
+   * Returns the {@link S3AsyncClient} that is configured for this {@link
+   * org.apache.iceberg.io.FileIO} instance.
+   *
+   * @deprecated since 1.10.0, will be removed in 1.11.0; use {@link
+   *     S3FileIO#clientForStoragePath(String)} instead.
+   */
+  @Deprecated
+  public S3AsyncClient asyncClient() {
+    return clientForStoragePath("s3").s3Async();
+  }
+
+  public PrefixedS3Client clientForStoragePath(String storagePath) {
+    PrefixedS3Client client;
+    String matchingPrefix = "s3";
+
+    for (String storagePrefix : s3ClientCache().asMap().keySet()) {
+      if (storagePath.startsWith(storagePrefix)
+          && storagePrefix.length() > matchingPrefix.length()) {
+        matchingPrefix = storagePrefix;
       }
     }
+
+    client = s3ClientCache().getIfPresent(matchingPrefix);
+
+    Preconditions.checkState(
+        null != client, "[BUG] S3 client for storage path not available: %s", 
storagePath);
     return client;
   }
 
-  public S3AsyncClient asyncClient() {
-    if (asyncClient == null) {
+  private Cache<String, PrefixedS3Client> s3ClientCache() {
+    if (null == s3ClientCache) {
       synchronized (this) {
-        if (asyncClient == null) {
-          asyncClient = s3Async.get();
+        if (null == s3ClientCache) {
+          this.s3ClientCache =

Review Comment:
   See my comments on the GCS version of this, I don't think we need to use a 
cache for this.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to