JCLOUDS-1371: Optimize filesystem delimiter

populateBlobKeysInContainer will no longer recurse when the delimiter
matches "/".  This makes listing deep hierarchies with a delimiter
faster.  Note that the general LocalBlobStore handling is still
required for the general cases.  This requires removing a bogus test
case.  References gaul/s3proxy#473.
This commit is contained in:
Andrew Gaul 2023-01-22 16:44:09 +09:00 committed by Andrew Gaul
parent e478dd5452
commit 62632c9db6
6 changed files with 18 additions and 36 deletions

View File

@ -342,7 +342,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
* @throws IOException
*/
@Override
public Iterable<String> getBlobKeysInsideContainer(String container, String prefix) throws IOException {
public Iterable<String> getBlobKeysInsideContainer(String container, String prefix, String delimiter) throws IOException {
filesystemContainerNameValidator.validate(container);
// check if container exists
// TODO maybe an error is more appropriate
@ -361,7 +361,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
}
}
populateBlobKeysInContainer(containerFile, blobNames, prefix, new Function<String, String>() {
populateBlobKeysInContainer(containerFile, blobNames, prefix, delimiter, new Function<String, String>() {
@Override
public String apply(String string) {
return denormalize(string.substring(containerPathLength));
@ -464,6 +464,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
.contentType(contentType)
.expires(expires)
.tier(tier)
.type(isDirectory ? StorageType.FOLDER : StorageType.BLOB)
.userMetadata(userMetadata.build());
} else {
builder.payload(byteSource)
@ -770,7 +771,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
public long countBlobs(String container, ListContainerOptions options) {
// TODO: honor options
try {
return Iterables.size(getBlobKeysInsideContainer(container, null));
return Iterables.size(getBlobKeysInsideContainer(container, null, null));
} catch (IOException ioe) {
throw Throwables.propagate(ioe);
}
@ -981,7 +982,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
}
private static void populateBlobKeysInContainer(File directory, Set<String> blobNames,
String prefix, Function<String, String> function) {
String prefix, String delimiter, Function<String, String> function) {
File[] children = directory.listFiles();
if (children == null) {
return;
@ -1001,7 +1002,11 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
continue;
}
blobNames.add(fullPath + File.separator); // TODO: undo if failures
populateBlobKeysInContainer(child, blobNames, prefix, function);
// Skip recursion if the delimiter tells us not to return children.
if (delimiter != null && delimiter.equals("/")) {
continue;
}
populateBlobKeysInContainer(child, blobNames, prefix, delimiter, function);
}
}
}

View File

@ -64,7 +64,6 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.io.ByteSource;
import com.google.common.io.Files;
@ -527,20 +526,6 @@ public class FilesystemBlobStoreTest {
blobKey.substring(0, blobKey.length() - 1)));
}
@Test(dataProvider = "ignoreOnMacOSX")
public void testListDirectoryBlobs() {
blobStore.createContainerInLocation(null, CONTAINER_NAME);
checkForContainerContent(CONTAINER_NAME, null);
Set<String> dirs = ImmutableSet.of(TestUtils.createRandomBlobKey("directory-", File.separator));
for (String d : dirs) {
blobStore.putBlob(CONTAINER_NAME, createDirBlob(d));
assertTrue(blobStore.blobExists(CONTAINER_NAME, d));
}
checkForContainerContent(CONTAINER_NAME, dirs);
}
@Test(dataProvider = "ignoreOnMacOSX")
public void testListDirectoryBlobsS3FS() {
blobStore.createContainerInLocation(null, CONTAINER_NAME);

View File

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

View File

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

View File

@ -150,7 +150,7 @@ public class TransientStorageStrategy implements LocalStorageStrategy {
}
@Override
public Iterable<String> getBlobKeysInsideContainer(final String containerName, String prefix) {
public Iterable<String> getBlobKeysInsideContainer(final String containerName, String prefix, String delimiter) {
ConcurrentSkipListMap<String, Blob> blobs = containerToBlobs.get(containerName);
if (prefix == null) {
return blobs.keySet();

View File

@ -238,20 +238,12 @@ public final class LocalBlobStore implements BlobStore {
// Loading blobs from container
Iterable<String> blobBelongingToContainer = null;
try {
blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(containerName, options.getPrefix());
blobBelongingToContainer = storageStrategy.getBlobKeysInsideContainer(containerName, options.getPrefix(), options.getDelimiter());
} catch (IOException e) {
logger.error(e, "An error occurred loading blobs contained into container %s", containerName);
propagate(e);
}
blobBelongingToContainer = Iterables.filter(blobBelongingToContainer,
new Predicate<String>() {
@Override
public boolean apply(String key) {
// ignore folders
return storageStrategy.blobExists(containerName, key);
}
});
SortedSet<StorageMetadata> contents = newTreeSet(FluentIterable.from(blobBelongingToContainer)
.transform(new Function<String, StorageMetadata>() {
@Override
@ -414,7 +406,7 @@ public final class LocalBlobStore implements BlobStore {
boolean returnVal = true;
if (storageStrategy.containerExists(containerName)) {
try {
if (Iterables.isEmpty(storageStrategy.getBlobKeysInsideContainer(containerName, null)))
if (Iterables.isEmpty(storageStrategy.getBlobKeysInsideContainer(containerName, null, null)))
storageStrategy.deleteContainer(containerName);
else
returnVal = false;