JCLOUDS-1371: JCLOUDS-1488: list optimize prefix

Previously getBlobKeysInsideContainer returned all keys and filtered
in LocalBlobStore.  Now getBlobKeysInsideContainer filters via prefix
which can dramatically decrease the number of keys returned,
especially for the filesystem provider.  Further optimizations are
possible for delimiter.
This commit is contained in:
Andrew Gaul 2019-01-25 11:42:13 -08:00
parent 3135aca109
commit 29eec441e9
5 changed files with 34 additions and 18 deletions

View File

@ -342,7 +342,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
* @throws IOException * @throws IOException
*/ */
@Override @Override
public Iterable<String> getBlobKeysInsideContainer(String container) throws IOException { public Iterable<String> getBlobKeysInsideContainer(String container, String prefix) throws IOException {
filesystemContainerNameValidator.validate(container); filesystemContainerNameValidator.validate(container);
// check if container exists // check if container exists
// TODO maybe an error is more appropriate // TODO maybe an error is more appropriate
@ -353,7 +353,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
File containerFile = openFolder(container); File containerFile = openFolder(container);
final int containerPathLength = containerFile.getAbsolutePath().length() + 1; final int containerPathLength = containerFile.getAbsolutePath().length() + 1;
populateBlobKeysInContainer(containerFile, blobNames, new Function<String, String>() { populateBlobKeysInContainer(containerFile, blobNames, prefix, new Function<String, String>() {
@Override @Override
public String apply(String string) { public String apply(String string) {
return denormalize(string.substring(containerPathLength)); return denormalize(string.substring(containerPathLength));
@ -753,7 +753,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
public long countBlobs(String container, ListContainerOptions options) { public long countBlobs(String container, ListContainerOptions options) {
// TODO: honor options // TODO: honor options
try { try {
return Iterables.size(getBlobKeysInsideContainer(container)); return Iterables.size(getBlobKeysInsideContainer(container, null));
} catch (IOException ioe) { } catch (IOException ioe) {
throw Throwables.propagate(ioe); throw Throwables.propagate(ioe);
} }
@ -964,17 +964,27 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
} }
private static void populateBlobKeysInContainer(File directory, Set<String> blobNames, private static void populateBlobKeysInContainer(File directory, Set<String> blobNames,
Function<String, String> function) { String prefix, Function<String, String> function) {
File[] children = directory.listFiles(); File[] children = directory.listFiles();
if (children == null) { if (children == null) {
return; return;
} }
for (File child : children) { for (File child : children) {
String fullPath = function.apply(child.getAbsolutePath());
if (child.isFile()) { if (child.isFile()) {
blobNames.add( function.apply(child.getAbsolutePath()) ); if (prefix != null && !fullPath.startsWith(prefix)) {
continue;
}
blobNames.add(fullPath);
} else if (child.isDirectory()) { } else if (child.isDirectory()) {
blobNames.add(function.apply(child.getAbsolutePath()) + File.separator); // TODO: undo if failures // Consider a prefix /a/b/c but we have only descended to path /a.
populateBlobKeysInContainer(child, blobNames, function); // We need to match the path against the prefix to continue
// matching down to /a/b.
if (prefix != null && !fullPath.startsWith(prefix) && !prefix.startsWith(fullPath + "/")) {
continue;
}
blobNames.add(fullPath + File.separator); // TODO: undo if failures
populateBlobKeysInContainer(child, blobNames, prefix, function);
} }
} }
} }

View File

@ -427,7 +427,7 @@ public class FilesystemStorageStrategyImplTest {
Blob blob = storageStrategy.newBlob(blobKey); Blob blob = storageStrategy.newBlob(blobKey);
storageStrategy.putBlob(CONTAINER_NAME, blob); storageStrategy.putBlob(CONTAINER_NAME, blob);
Iterable<String> keys = storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME); Iterable<String> keys = storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME, null);
Iterator<String> iter = keys.iterator(); Iterator<String> iter = keys.iterator();
assertTrue(iter.hasNext()); assertTrue(iter.hasNext());
assertEquals(iter.next(), blobKey); assertEquals(iter.next(), blobKey);
@ -598,7 +598,7 @@ public class FilesystemStorageStrategyImplTest {
Iterable<String> resultList; Iterable<String> resultList;
// no container // no container
resultList = storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME); resultList = storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME, null);
assertNotNull(resultList, "Result is null"); assertNotNull(resultList, "Result is null");
assertFalse(resultList.iterator().hasNext(), "Blobs detected"); assertFalse(resultList.iterator().hasNext(), "Blobs detected");
@ -609,10 +609,10 @@ public class FilesystemStorageStrategyImplTest {
TestUtils.createRandomBlobKey("GetBlobKeys-", ".jpg"), TestUtils.createRandomBlobKey("GetBlobKeys-", ".jpg"),
TestUtils.createRandomBlobKey("563" + "/" + "g3sx2" + "/" + "removeBlob-", ".jpg"), TestUtils.createRandomBlobKey("563" + "/" + "g3sx2" + "/" + "removeBlob-", ".jpg"),
TestUtils.createRandomBlobKey("563" + "/" + "g3sx2" + "/" + "removeBlob-", ".jpg") }); TestUtils.createRandomBlobKey("563" + "/" + "g3sx2" + "/" + "removeBlob-", ".jpg") });
storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME); storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME, null);
List<String> retrievedBlobKeys = Lists.newArrayList(); List<String> retrievedBlobKeys = Lists.newArrayList();
resultList = storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME); resultList = storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME, null);
Iterator<String> containersIterator = resultList.iterator(); Iterator<String> containersIterator = resultList.iterator();
while (containersIterator.hasNext()) { while (containersIterator.hasNext()) {
retrievedBlobKeys.add(containersIterator.next()); retrievedBlobKeys.add(containersIterator.next());

View File

@ -98,7 +98,7 @@ public interface LocalStorageStrategy {
* @return * @return
* @throws IOException * @throws IOException
*/ */
Iterable<String> getBlobKeysInsideContainer(String container) throws IOException; Iterable<String> getBlobKeysInsideContainer(String container, String prefix) throws IOException;
/** /**
* Load the blob with the given key belonging to the container with the given * Load the blob with the given key belonging to the container with the given

View File

@ -25,6 +25,7 @@ import java.util.Date;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListMap;
import javax.inject.Inject; import javax.inject.Inject;
@ -59,7 +60,7 @@ import com.google.common.io.ByteStreams;
import com.google.common.net.HttpHeaders; import com.google.common.net.HttpHeaders;
public class TransientStorageStrategy implements LocalStorageStrategy { public class TransientStorageStrategy implements LocalStorageStrategy {
private final ConcurrentMap<String, ConcurrentMap<String, Blob>> containerToBlobs = new ConcurrentHashMap<String, ConcurrentMap<String, Blob>>(); private final ConcurrentMap<String, ConcurrentSkipListMap<String, Blob>> containerToBlobs = new ConcurrentHashMap<String, ConcurrentSkipListMap<String, Blob>>();
private final ConcurrentMap<String, ConcurrentMap<String, BlobAccess>> containerToBlobAccess = new ConcurrentHashMap<String, ConcurrentMap<String, BlobAccess>>(); private final ConcurrentMap<String, ConcurrentMap<String, BlobAccess>> containerToBlobAccess = new ConcurrentHashMap<String, ConcurrentMap<String, BlobAccess>>();
private final ConcurrentMap<String, StorageMetadata> containerMetadata = new ConcurrentHashMap<String, StorageMetadata>(); private final ConcurrentMap<String, StorageMetadata> containerMetadata = new ConcurrentHashMap<String, StorageMetadata>();
private final ConcurrentMap<String, ContainerAccess> containerAccessMap = new ConcurrentHashMap<String, ContainerAccess>(); private final ConcurrentMap<String, ContainerAccess> containerAccessMap = new ConcurrentHashMap<String, ContainerAccess>();
@ -90,7 +91,7 @@ public class TransientStorageStrategy implements LocalStorageStrategy {
@Override @Override
public boolean createContainerInLocation(String containerName, Location location, CreateContainerOptions options) { public boolean createContainerInLocation(String containerName, Location location, CreateContainerOptions options) {
ConcurrentMap<String, Blob> origValue = containerToBlobs.putIfAbsent( ConcurrentMap<String, Blob> origValue = containerToBlobs.putIfAbsent(
containerName, new ConcurrentHashMap<String, Blob>()); containerName, new ConcurrentSkipListMap<String, Blob>());
if (origValue != null) { if (origValue != null) {
return false; return false;
} }
@ -148,8 +149,13 @@ public class TransientStorageStrategy implements LocalStorageStrategy {
} }
@Override @Override
public Iterable<String> getBlobKeysInsideContainer(final String containerName) { public Iterable<String> getBlobKeysInsideContainer(final String containerName, String prefix) {
return containerToBlobs.get(containerName).keySet(); ConcurrentSkipListMap<String, Blob> blobs = containerToBlobs.get(containerName);
if (prefix == null) {
return blobs.keySet();
}
String lastPrefix = prefix + (char) 65535; // TODO: better sentinel?
return blobs.subMap(prefix, /*fromInclusive=*/ true, lastPrefix, /*toInclusive=*/ false).keySet();
} }
@Override @Override

View File

@ -238,7 +238,7 @@ public final class LocalBlobStore implements BlobStore {
// Loading blobs from container // Loading blobs from container
Iterable<String> blobBelongingToContainer = null; Iterable<String> blobBelongingToContainer = null;
try { try {
blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(containerName); blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(containerName, options.getPrefix());
} catch (IOException e) { } catch (IOException e) {
logger.error(e, "An error occurred loading blobs contained into container %s", containerName); logger.error(e, "An error occurred loading blobs contained into container %s", containerName);
propagate(e); propagate(e);
@ -414,7 +414,7 @@ public final class LocalBlobStore implements BlobStore {
boolean returnVal = true; boolean returnVal = true;
if (storageStrategy.containerExists(containerName)) { if (storageStrategy.containerExists(containerName)) {
try { try {
if (Iterables.isEmpty(storageStrategy.getBlobKeysInsideContainer(containerName))) if (Iterables.isEmpty(storageStrategy.getBlobKeysInsideContainer(containerName, null)))
storageStrategy.deleteContainer(containerName); storageStrategy.deleteContainer(containerName);
else else
returnVal = false; returnVal = false;