david-streamlio commented on a change in pull request #2865: Refactored JCloud
Tiered Storage
URL: https://github.com/apache/pulsar/pull/2865#discussion_r238918485
##########
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:
That's why I had to the BlobStoreRepository class to keep these, and be able
to look them up based on the provider, region, endpoint parameters passed in.
There is a similar map of these bookstores in the current version of the
BlobStoreManagedLedgerOffloader class I am refactoring.
private final ConcurrentMap<BlobStoreLocation, BlobStore> readBlobStores =
new ConcurrentHashMap<>();
If I cannot keep these blobstore instances somewhere, then I cannot fix any
of the unit tests. So if removing the BlobStoreRepository is a MUST have, then
let's just kill this PR and keep the current implementation, as I don't see any
other way of fixing the unit tests.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
