From 9ffc45d0f140bd1b5d3897a3c3e5d041d99dba52 Mon Sep 17 00:00:00 2001 From: Shri Javadekar Date: Wed, 24 Apr 2013 13:05:11 -0700 Subject: [PATCH] Use ConcurrentSkipListMap for transient blobstore. The current implementation uses a ConcurrentHashMap for storing key value pairs in the Transient blobstore. LocalAsyncBlobStore.list has to fetch all the blobs from this map and then return a subset based on the marker. Instead this change uses a ConcurrentSkipListMap which has natural ordering and sends the marker down to TransientStorageStrategy for more efficient enumeration. Fixes #1561. --- .../FilesystemStorageStrategyImpl.java | 23 +++++++++++++------ .../blobstore/LocalAsyncBlobStore.java | 7 +++++- .../blobstore/LocalStorageStrategy.java | 12 +++++++++- .../blobstore/TransientStorageStrategy.java | 12 ++++++++-- 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java index eb38ddcb02..c8d5eb5b65 100644 --- a/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java +++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java @@ -24,7 +24,8 @@ import java.io.File; import java.io.IOException; -import java.util.Set; +import java.util.Collection; +import java.util.List; import javax.annotation.Resource; import javax.inject.Inject; @@ -42,13 +43,14 @@ import org.jclouds.filesystem.util.Utils; import org.jclouds.io.Payload; import org.jclouds.io.Payloads; +import org.jclouds.javax.annotation.Nullable; import org.jclouds.logging.Logger; import org.jclouds.rest.annotations.ParamValidators; import com.google.common.base.Function; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Sets; +import com.google.common.collect.Lists; import com.google.common.io.ByteStreams; import com.google.common.io.Files; @@ -143,7 +145,14 @@ public boolean blobExists(String container, String key) { filesystemBlobKeyValidator.validate(key); return buildPathAndChecksIfFileExists(container, key); } - + + @Override + public Iterable getBlobKeysInsideContainer(String container, String fromElement) + throws IOException { + ImmutableList blobNames = ImmutableList.copyOf(getBlobKeysInsideContainer(container)); + return blobNames.subList(blobNames.indexOf(fromElement), blobNames.size()); + } + /** * Returns all the blobs key inside a container * @@ -156,11 +165,11 @@ public Iterable getBlobKeysInsideContainer(String container) throws IOEx filesystemContainerNameValidator.validate(container); // check if container exists // TODO maybe an error is more appropriate - Set blobNames = Sets.newHashSet(); + List blobNames = Lists.newArrayList(); if (!containerExists(container)) { return blobNames; } - + File containerFile = openFolder(container); final int containerPathLength = containerFile.getAbsolutePath().length() + 1; populateBlobKeysInContainer(containerFile, blobNames, new Function() { @@ -169,6 +178,7 @@ public String apply(String string) { return string.substring(containerPathLength); } }); + return blobNames; } @@ -435,7 +445,7 @@ private File openFolder(String folderName) throws IOException { return folder; } - private static void populateBlobKeysInContainer(File directory, Set blobNames, + private static void populateBlobKeysInContainer(File directory, Collection blobNames, Function function) { File[] children = directory.listFiles(); for (File child : children) { @@ -469,5 +479,4 @@ protected boolean createDirectoryWithResult(String container, String directory) boolean result = directoryToCreate.mkdirs(); return result; } - } diff --git a/blobstore/src/main/java/org/jclouds/blobstore/LocalAsyncBlobStore.java b/blobstore/src/main/java/org/jclouds/blobstore/LocalAsyncBlobStore.java index 019a891033..a25e8eadc7 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/LocalAsyncBlobStore.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/LocalAsyncBlobStore.java @@ -133,7 +133,12 @@ public ListenableFuture> list(final String co // Loading blobs from container Iterable blobBelongingToContainer = null; try { - blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(container); + String marker = options.getMarker(); + if (marker != null) { + blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(container, marker); + } else { + blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(container); + } } catch (IOException e) { logger.error(e, "An error occurred loading blobs contained into container %s", container); Throwables.propagate(e); diff --git a/blobstore/src/main/java/org/jclouds/blobstore/LocalStorageStrategy.java b/blobstore/src/main/java/org/jclouds/blobstore/LocalStorageStrategy.java index d37fdf8b50..9a60045aa6 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/LocalStorageStrategy.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/LocalStorageStrategy.java @@ -23,6 +23,7 @@ import org.jclouds.blobstore.domain.Blob; import org.jclouds.blobstore.options.ListContainerOptions; import org.jclouds.domain.Location; +import org.jclouds.javax.annotation.Nullable; /** * Strategy for local operations related to container and blob @@ -85,13 +86,22 @@ public interface LocalStorageStrategy { boolean blobExists(String container, String key); /** - * Returns all the blobs key inside a container + * Returns all the blob keys inside a container * @param container * @return * @throws IOException */ Iterable getBlobKeysInsideContainer(String container) throws IOException; + /** + * Returns the blob keys inside a container starting from a particular element + * @param container + * @param fromElement + * @return + * @throws IOException + */ + Iterable getBlobKeysInsideContainer(String container, String fromElement) throws IOException; + /** * Load the blob with the given key belonging to the container with the given * name. There must exist a resource on the file system whose complete name diff --git a/blobstore/src/main/java/org/jclouds/blobstore/TransientStorageStrategy.java b/blobstore/src/main/java/org/jclouds/blobstore/TransientStorageStrategy.java index 2d6b71ebe5..9b9111a4e6 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/TransientStorageStrategy.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/TransientStorageStrategy.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ConcurrentSkipListMap; import javax.inject.Inject; import javax.ws.rs.core.HttpHeaders; @@ -45,6 +46,7 @@ import org.jclouds.io.Payloads; import org.jclouds.io.payloads.ByteArrayPayload; import org.jclouds.io.payloads.DelegatingPayload; +import org.jclouds.javax.annotation.Nullable; import com.google.common.base.Supplier; import com.google.common.base.Throwables; @@ -52,7 +54,8 @@ import com.google.common.collect.Multimaps; public class TransientStorageStrategy implements LocalStorageStrategy { - private final ConcurrentMap> containerToBlobs = new ConcurrentHashMap>(); + private final ConcurrentMap> containerToBlobs = + new ConcurrentHashMap>(); private final ConcurrentMap containerToLocation = new ConcurrentHashMap(); private final Supplier defaultLocation; private final DateService dateService; @@ -81,7 +84,7 @@ public Iterable getAllContainerNames() { @Override public boolean createContainerInLocation(final String containerName, final Location location) { ConcurrentMap origValue = containerToBlobs.putIfAbsent( - containerName, new ConcurrentHashMap()); + containerName, new ConcurrentSkipListMap()); if (origValue != null) { return false; } @@ -116,6 +119,11 @@ public Iterable getBlobKeysInsideContainer(final String containerName) { return containerToBlobs.get(containerName).keySet(); } + @Override + public Iterable getBlobKeysInsideContainer(final String containerName, final String fromElement) { + return containerToBlobs.get(containerName).keySet().tailSet(fromElement); + } + @Override public Blob getBlob(final String containerName, final String blobName) { Map map = containerToBlobs.get(containerName);