From 4ccd0239222ea1ef33c4acc515e34fe6ec8b9089 Mon Sep 17 00:00:00 2001
From: Piotr Findeisen <piotr.findeisen@gmail.com>
Date: Wed, 2 Aug 2023 11:12:33 +0200
Subject: [PATCH] Add diagnostic logging for TrinoS3FileSystem delete

---
 .../src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java b/lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java
index 04b5c6df8309..87deecddb1e9 100644
--- a/lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java
+++ b/lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java
@@ -533,6 +533,7 @@ private static boolean isDirectoryMediaType(String contentType)
             return MediaType.parse(contentType).is(DIRECTORY_MEDIA_TYPE);
         }
         catch (IllegalArgumentException e) {
+            log.debug(e, "isDirectoryMediaType: failed to inspect contentType [%s], assuming not a directory", contentType);
             return false;
         }
     }
@@ -652,6 +653,7 @@ public boolean rename(Path src, Path dst)
             delete(src, true);
         }
 
+        // TODO should we return true also when deleteObject() returned false?
         return true;
     }
 
@@ -695,6 +697,7 @@ else if (deletePrefixResult == DeletePrefixResult.DELETE_KEYS_FAILURE) {
             }
             deleteObject(key + DIRECTORY_SUFFIX);
         }
+        // TODO should we return true also when deleteObject() returned false? (currently deleteObject's return value is never used)
         return true;
     }
 
@@ -744,8 +747,9 @@ private boolean directory(Path path)
 
     private boolean deleteObject(String key)
     {
+        String bucketName = getBucketName(uri);
         try {
-            DeleteObjectRequest deleteObjectRequest = new DeleteObjectRequest(getBucketName(uri), key);
+            DeleteObjectRequest deleteObjectRequest = new DeleteObjectRequest(bucketName, key);
             if (requesterPaysEnabled) {
                 // TODO use deleteObjectRequest.setRequesterPays() when https://github.com/aws/aws-sdk-java/issues/1219 is fixed
                 // currently the method exists, but is ineffective (doesn't set the required HTTP header)
@@ -756,6 +760,8 @@ private boolean deleteObject(String key)
             return true;
         }
         catch (AmazonClientException e) {
+            // TODO should we propagate this?
+            log.debug(e, "Failed to delete object from the bucket %s: %s", bucketName, key);
             return false;
         }
     }