From e4514240e09fc12b43bdde078826edd10e80d74d Mon Sep 17 00:00:00 2001 From: Andrew Gaul Date: Thu, 26 Apr 2012 23:11:42 -0700 Subject: [PATCH] Optimize clearContainer for large folders Previously we built up a list of all blobs then executed removeBlob on all of them simultaneously. For sufficiently large folders this would cause an OutOfMemoryError. There are cleaner ways to write this but this approach mimimizes the patch. I will continue working on this in subsequent commits, including optimizing for deep subdirectories. This commit also fixes a bug in the transient blobstore where deleting elements while reading a directory resulted in NoSuchElementException. --- .../blobstore/TransientAsyncBlobStore.java | 6 ++- .../internal/DeleteAllKeysInList.java | 50 +++++++++++++------ 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java b/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java index 81ef2f5dfd..28891d32a3 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java @@ -179,11 +179,13 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { final String finalMarker = options.getMarker(); StorageMetadata lastMarkerMetadata = find(contents, new Predicate() { public boolean apply(StorageMetadata metadata) { - return metadata.getName().equals(finalMarker); + return metadata.getName().compareTo(finalMarker) >= 0; } }); contents = contents.tailSet(lastMarkerMetadata); - contents.remove(lastMarkerMetadata); + if (finalMarker.equals(lastMarkerMetadata.getName())) { + contents.remove(lastMarkerMetadata); + } } final String prefix = options.getDir(); diff --git a/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java b/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java index ee4c38ea05..dc8b816df2 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java @@ -22,6 +22,7 @@ import static org.jclouds.blobstore.options.ListContainerOptions.Builder.recursi import static org.jclouds.concurrent.FutureIterables.awaitCompletion; import java.util.Map; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -31,17 +32,18 @@ import javax.inject.Singleton; import org.jclouds.Constants; import org.jclouds.blobstore.AsyncBlobStore; +import org.jclouds.blobstore.domain.PageSet; import org.jclouds.blobstore.domain.StorageMetadata; import org.jclouds.blobstore.internal.BlobRuntimeException; import org.jclouds.blobstore.options.ListContainerOptions; import org.jclouds.blobstore.reference.BlobStoreConstants; import org.jclouds.blobstore.strategy.ClearContainerStrategy; import org.jclouds.blobstore.strategy.ClearListStrategy; -import org.jclouds.blobstore.strategy.ListContainerStrategy; import org.jclouds.http.handlers.BackoffLimitedRetryHandler; import org.jclouds.logging.Logger; import com.google.common.base.Predicate; +import com.google.common.base.Throwables; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.inject.Inject; @@ -57,7 +59,6 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr @Named(BlobStoreConstants.BLOBSTORE_LOGGER) protected Logger logger = Logger.NULL; - protected final ListContainerStrategy listContainer; protected final BackoffLimitedRetryHandler retryHandler; private final ExecutorService userExecutor; @@ -71,12 +72,11 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr @Inject DeleteAllKeysInList(@Named(Constants.PROPERTY_USER_THREADS) ExecutorService userExecutor, - AsyncBlobStore connection, ListContainerStrategy listContainer, + AsyncBlobStore connection, BackoffLimitedRetryHandler retryHandler) { this.userExecutor = userExecutor; this.connection = connection; - this.listContainer = listContainer; this.retryHandler = retryHandler; } @@ -84,19 +84,31 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr execute(containerName, recursive()); } - public void execute(final String containerName, final ListContainerOptions options) { + public void execute(final String containerName, ListContainerOptions options) { String message = options.getDir() != null ? String.format("clearing path %s/%s", containerName, options.getDir()) : String.format("clearing container %s", containerName); + options = options.clone(); if (options.isRecursive()) message = message + " recursively"; Map exceptions = Maps.newHashMap(); + PageSet listing; Iterable toDelete; - for (int i = 0; i < 3; i++) { // TODO parameterize - toDelete = getResourcesToDelete(containerName, options); - if (Iterables.isEmpty(toDelete)) { - break; + int maxErrors = 3; // TODO parameterize + for (int i = 0; i < maxErrors; ) { + try { + listing = connection.list(containerName, options).get(); + } catch (ExecutionException ee) { + ++i; + if (i == maxErrors) { + throw new BlobRuntimeException("list error", ee.getCause()); + } + retryHandler.imposeBackoffExponentialDelay(i, message); + continue; + } catch (InterruptedException ie) { + throw Throwables.propagate(ie); } + toDelete = filterListing(listing, options); Map> responses = Maps.newHashMap(); try { @@ -127,24 +139,30 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr exceptions = awaitCompletion(responses, userExecutor, maxTime, logger, message); } if (!exceptions.isEmpty()) { - retryHandler.imposeBackoffExponentialDelay(i + 1, message); + ++i; + retryHandler.imposeBackoffExponentialDelay(i, message); + continue; } + + String marker = listing.getNextMarker(); + if (marker == null) { + break; + } + options = options.afterMarker(marker); } if (!exceptions.isEmpty()) throw new BlobRuntimeException(String.format("error %s: %s", message, exceptions)); - toDelete = getResourcesToDelete(containerName, options); - assert Iterables.isEmpty(toDelete) : String.format("items remaining %s: %s", message, - toDelete); } private boolean parentIsFolder(final ListContainerOptions options, final StorageMetadata md) { return (options.getDir() != null && md.getName().indexOf('/') == -1); } - private Iterable getResourcesToDelete(final String containerName, + private Iterable filterListing( + final PageSet listing, final ListContainerOptions options) { - Iterable toDelete = Iterables.filter(listContainer.execute( - containerName, options), new Predicate() { + Iterable toDelete = Iterables.filter(listing, + new Predicate() { @Override public boolean apply(StorageMetadata input) {