feat!: Improve file attribute caching (#854)

BREAKING CHANGE: `S3OutputStream` constructor signatures changed
carlspring · Nov 5, 2024 · 154d72a · 154d72a
1 parent 08d14c2
commit 154d72a
Show file tree

Hide file tree

Showing 12 changed files with 348 additions and 161 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -15,7 +15,7 @@ plugins {
 
 allprojects {
     repositories {
-        mavenLocal()
+        //mavenLocal()
         // Allows you to specify your own repository manager instance.
         if (project.hasProperty("s3fs.proxy.url")) {
             maven {

diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3Factory.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3Factory.java
@@ -47,11 +47,13 @@ public abstract class S3Factory
      * Maximum TTL in millis to cache {@link S3BasicFileAttributes} and {@link S3PosixFileAttributes}.
      */
     public static final String CACHE_ATTRIBUTES_TTL = "s3fs.cache.attributes.ttl";
+    public static final int CACHE_ATTRIBUTES_TTL_DEFAULT = 60000;
 
     /**
      * Total size of {@link S3BasicFileAttributes} and {@link S3PosixFileAttributes} cache.
      */
     public static final String CACHE_ATTRIBUTES_SIZE = "s3fs.cache.attributes.size";
+    public static final int CACHE_ATTRIBUTES_SIZE_DEFAULT = 5000;
 
     public static final String REQUEST_METRIC_COLLECTOR_CLASS = "s3fs.request.metric.collector.class";
 

diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystem.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystem.java
@@ -53,8 +53,8 @@ public S3FileSystem(final S3FileSystemProvider provider,
         this.endpoint = endpoint;
         this.properties = properties;
 
-        int cacheTTL = Integer.parseInt(String.valueOf(properties.getOrDefault(S3Factory.CACHE_ATTRIBUTES_TTL, "60000")));
-        int cacheSize = Integer.parseInt(String.valueOf(properties.getOrDefault(S3Factory.CACHE_ATTRIBUTES_SIZE, "5000")));
+        int cacheTTL = Integer.parseInt(String.valueOf(properties.getOrDefault(S3Factory.CACHE_ATTRIBUTES_TTL, S3Factory.CACHE_ATTRIBUTES_TTL_DEFAULT)));
+        int cacheSize = Integer.parseInt(String.valueOf(properties.getOrDefault(S3Factory.CACHE_ATTRIBUTES_SIZE, S3Factory.CACHE_ATTRIBUTES_SIZE_DEFAULT)));
 
         this.fileAttributesCache = new S3FileAttributesCache(cacheTTL, cacheSize);
     }

diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProvider.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3FileSystemProvider.java
@@ -543,7 +543,7 @@ public OutputStream newOutputStream(final Path path,
 
         S3FileSystem fileSystem = s3Path.getFileSystem();
 
-        return new S3OutputStream(fileSystem.getClient(), s3Path.toS3ObjectId(), null, metadata, fileSystem.getRequestHeaderCacheControlProperty());
+        return new S3OutputStream(fileSystem.getClient(), s3Path.toS3ObjectId(), null, metadata, fileSystem.getRequestHeaderCacheControlProperty(), s3Path.getFileAttributesCache());
     }
 
     private void validateCreateAndTruncateOptions(final Path path,
@@ -644,9 +644,12 @@ public void createDirectory(Path dir,
                                                          .key(directoryKey)
                                                          .cacheControl(s3Path.getFileSystem().getRequestHeaderCacheControlProperty())
                                                          .contentLength(0L)
+                                                         .contentType("application/x-directory")
                                                          .build();
 
-        client.putObject(request, RequestBody.fromBytes(new byte[0]));
+        client.putObject(request, RequestBody.empty());
+
+        s3Path.getFileAttributesCache().invalidate(s3Path);
     }
 
     @Override
@@ -693,8 +696,7 @@ private void deleteBatch(S3Client client,
         try {
             client.deleteObjects(multiObjectDeleteRequest);
             for (S3Path path : batch) {
-                path.getFileAttributesCache().invalidate(path, BasicFileAttributes.class);
-                path.getFileAttributesCache().invalidate(path, PosixFileAttributes.class);
+                path.getFileAttributesCache().invalidate(path);
             }
         } catch (SdkException e) {
             throw new IOException(e);
@@ -810,6 +812,9 @@ public void copy(Path source,
                                                            .build();
 
         client.copyObject(request);
+
+        s3Source.getFileAttributesCache().invalidate(s3Source);
+        s3Source.getFileAttributesCache().invalidate(s3Target);
     }
 
     private String encodeUrl(final String bucketNameOrigin,
@@ -1100,30 +1105,10 @@ boolean exists(S3Path path)
     {
         S3Path s3Path = toS3Path(path);
 
-        if (isBucketRoot(s3Path))
-        {
-            // check to see if bucket exists
-            try
-            {
-                s3Utils.listS3Objects(s3Path);
-                return true;
-            }
-            catch (SdkException e)
-            {
-                return false;
-            }
-        }
-
-        try
-        {
-            s3Utils.getS3Object(s3Path);
+        S3BasicFileAttributes attrs = s3Path.getFileAttributesCache()
+                                            .get(s3Path, BasicFileAttributes.class);
 
-            return true;
-        }
-        catch (NoSuchFileException e)
-        {
-            return false;
-        }
+        return attrs != null;
     }
 
     public void close(S3FileSystem fileSystem)

diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3ObjectId.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3ObjectId.java
@@ -68,7 +68,7 @@ public String toString()
         return "bucket: " + bucket + ", key: " + key;
     }
 
-    static final class Builder
+    public static final class Builder
     {
 
         private String bucket;

diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3OutputStream.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3OutputStream.java
@@ -5,6 +5,8 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.SequenceInputStream;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.PosixFileAttributes;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -14,6 +16,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
+import org.carlspring.cloud.storage.s3fs.cache.S3FileAttributesCache;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.core.exception.SdkException;
@@ -75,6 +78,11 @@ public final class S3OutputStream
      */
     private final Map<String, String> metadata;
 
+    /**
+     * File attribute cache
+     */
+    private final S3FileAttributesCache fileAttributesCache;
+
     /**
      * Indicates if the stream has been closed.
      */
@@ -114,11 +122,7 @@ public final class S3OutputStream
     public S3OutputStream(final S3Client s3Client,
                           final S3ObjectId objectId)
     {
-        this.s3Client = requireNonNull(s3Client);
-        this.objectId = requireNonNull(objectId);
-        this.metadata = new HashMap<>();
-        this.storageClass = null;
-        this.requestCacheControlHeader = "";
+        this(s3Client, objectId, null, new HashMap<>(), "", new S3FileAttributesCache(S3Factory.CACHE_ATTRIBUTES_TTL_DEFAULT, S3Factory.CACHE_ATTRIBUTES_SIZE_DEFAULT));
     }
 
     /**
@@ -134,11 +138,7 @@ public S3OutputStream(final S3Client s3Client,
                           final S3ObjectId objectId,
                           final StorageClass storageClass)
     {
-        this.s3Client = requireNonNull(s3Client);
-        this.objectId = requireNonNull(objectId);
-        this.metadata = new HashMap<>();
-        this.storageClass = storageClass;
-        this.requestCacheControlHeader = "";
+        this(s3Client, objectId, storageClass, new HashMap<>(), "");
     }
 
     /**
@@ -155,11 +155,7 @@ public S3OutputStream(final S3Client s3Client,
                           final S3ObjectId objectId,
                           final Map<String, String> metadata)
     {
-        this.s3Client = requireNonNull(s3Client);
-        this.objectId = requireNonNull(objectId);
-        this.storageClass = null;
-        this.metadata = new HashMap<>(metadata);
-        this.requestCacheControlHeader = "";
+        this(s3Client, objectId, null, metadata, "", new S3FileAttributesCache(S3Factory.CACHE_ATTRIBUTES_TTL_DEFAULT, S3Factory.CACHE_ATTRIBUTES_SIZE_DEFAULT));
     }
 
     /**
@@ -177,11 +173,7 @@ public S3OutputStream(final S3Client s3Client,
                           final StorageClass storageClass,
                           final Map<String, String> metadata)
     {
-        this.s3Client = requireNonNull(s3Client);
-        this.objectId = requireNonNull(objectId);
-        this.storageClass = storageClass;
-        this.metadata = new HashMap<>(metadata);
-        this.requestCacheControlHeader = "";
+        this(s3Client, objectId, storageClass, metadata, "", new S3FileAttributesCache(S3Factory.CACHE_ATTRIBUTES_TTL_DEFAULT, S3Factory.CACHE_ATTRIBUTES_SIZE_DEFAULT));
     }
 
     /**
@@ -200,12 +192,34 @@ public S3OutputStream(final S3Client s3Client,
                           final StorageClass storageClass,
                           final Map<String, String> metadata,
                           final String requestCacheControlHeader)
+    {
+        this(s3Client, objectId, storageClass, metadata, requestCacheControlHeader, new S3FileAttributesCache(S3Factory.CACHE_ATTRIBUTES_TTL_DEFAULT, S3Factory.CACHE_ATTRIBUTES_SIZE_DEFAULT));
+    }
+
+    /**
+     * Creates a new {@code S3OutputStream} that writes data directly into the S3 object with the given {@code objectId}.
+     * The given {@code metadata} will be attached to the written object.
+     *
+     * @param s3Client     S3 ClientAPI to use
+     * @param objectId     ID of the S3 object to store data into
+     * @param storageClass S3 Client storage class to apply to the newly created S3 object, if any
+     * @param metadata     metadata to attach to the written object
+     * @param requestCacheControlHeader Controls
+     * @throws NullPointerException if at least one parameter except {@code storageClass} is {@code null}
+     */
+    public S3OutputStream(final S3Client s3Client,
+                          final S3ObjectId objectId,
+                          final StorageClass storageClass,
+                          final Map<String, String> metadata,
+                          final String requestCacheControlHeader,
+                          final S3FileAttributesCache fileAttributesCache)
     {
         this.s3Client = requireNonNull(s3Client);
         this.objectId = requireNonNull(objectId);
         this.storageClass = storageClass;
         this.metadata = new HashMap<>(metadata);
         this.requestCacheControlHeader = requestCacheControlHeader;
+        this.fileAttributesCache = fileAttributesCache;
     }
 
     //protected for testing purposes
@@ -308,10 +322,24 @@ public void close()
                 completeMultipartUpload();
             }
 
+            invalidateAttributeCache();
+
             closed.set(true);
         }
     }
 
+    @Override
+    public void flush() throws IOException
+    {
+        invalidateAttributeCache();
+    }
+
+    private void invalidateAttributeCache()
+    {
+        fileAttributesCache.invalidate(S3FileAttributesCache.generateCacheKey(objectId, BasicFileAttributes.class));
+        fileAttributesCache.invalidate(S3FileAttributesCache.generateCacheKey(objectId, PosixFileAttributes.class));
+    }
+
     /**
      * Creates a multipart upload and gets the upload id.
      *

diff --git a/src/main/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannel.java b/src/main/java/org/carlspring/cloud/storage/s3fs/S3SeekableByteChannel.java
@@ -184,6 +184,9 @@ protected void sync()
             S3Client client = path.getFileSystem().getClient();
 
             client.putObject(builder.build(), RequestBody.fromInputStream(stream, length));
+
+            // Invalidate cache as writing to the object would change some metadata.
+            path.getFileAttributesCache().invalidate(path);
         }
     }