[GitHub] ivankelly commented on a change in pull request #2865: Refactored JCloud Tiered Storage

[GitBox]

ivankelly commented on a change in pull request #2865: Refactored JCloud Tiered 
Storage
URL: https://github.com/apache/pulsar/pull/2865#discussion_r239084384
 
 

 ##########
 File path: 
tiered-storage/jcloud/src/main/java/org/apache/bookkeeper/mledger/offload/jcloud/impl/BlobStoreManagedLedgerOffloader.java
 ##########
 @@ -483,59 +227,37 @@ public String getOffloadDriverName() {
         return promise;
     }
 
-    String getReadRegion(Map<String, String> offloadDriverMetadata) {
-        return offloadDriverMetadata.getOrDefault(METADATA_FIELD_REGION, 
writeRegion);
-    }
-
-    String getReadBucket(Map<String, String> offloadDriverMetadata) {
-        return offloadDriverMetadata.getOrDefault(METADATA_FIELD_BUCKET, 
writeBucket);
-    }
-
-    String getReadEndpoint(Map<String, String> offloadDriverMetadata) {
-        return offloadDriverMetadata.getOrDefault(METADATA_FIELD_ENDPOINT, 
writeEndpoint);
-    }
-
-    BlobStore getReadBlobStore(Map<String, String> offloadDriverMetadata) {
-        BlobStoreLocation location = BlobStoreLocation.of(
-            getReadRegion(offloadDriverMetadata),
-            getReadEndpoint(offloadDriverMetadata)
-        );
-        BlobStore blobStore = readBlobStores.get(location);
-        if (null == blobStore) {
-            blobStore = createBlobStore(
-                offloadDriverName,
-                location.getRegion(),
-                location.getEndpoint(),
-                credentials,
-                maxBlockSize
-            ).getRight();
-            BlobStore existingBlobStore = readBlobStores.putIfAbsent(location, 
blobStore);
-            if (null == existingBlobStore) {
-                return blobStore;
-            } else {
-                return existingBlobStore;
-            }
-        } else {
-            return blobStore;
-        }
+    /**
+     * Attempts to create a BlobStoreLocation from the values in the 
offloadDriverMetadata,
+     * however, if no values are available, it defaults to the currently 
configured
+     * provider, region, bucket, etc.
+     *
+     * @param offloadDriverMetadata
+     * @return
+     */
+    private BlobStoreLocation getBlobStoreLocation(Map<String, String> 
offloadDriverMetadata) {
+        return (!offloadDriverMetadata.isEmpty()) ? new 
BlobStoreLocation(offloadDriverMetadata) :
+            new BlobStoreLocation(getOffloadDriverMetadata());
     }
 
     @Override
     public CompletableFuture<ReadHandle> readOffloaded(long ledgerId, UUID uid,
                                                        Map<String, String> 
offloadDriverMetadata) {
-        String readBucket = getReadBucket(offloadDriverMetadata);
-        BlobStore readBlobstore = getReadBlobStore(offloadDriverMetadata);
+
+        BlobStoreLocation bsKey = getBlobStoreLocation(offloadDriverMetadata);
+        String readBucket = bsKey.getBucket();
+        BlobStore readBlobstore = blobStore;
 
 Review comment:
   This unit test in particular are you referring to though?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services