Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop leaving leftover buckets in ITGcsNIO #3898

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,13 @@ public static void beforeClass() throws IOException {

@AfterClass
public static void afterClass() throws ExecutionException, InterruptedException {
if (storage != null && !RemoteStorageHelper.forceDelete(storage, BUCKET, 5, TimeUnit.SECONDS) &&
log.isLoggable(Level.WARNING)) {
log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", BUCKET);
if (storage != null) {
for (String bucket : new String[]{BUCKET, REQUESTER_PAYS_BUCKET}) {
if (!RemoteStorageHelper.forceDelete(storage, bucket, 5, TimeUnit.SECONDS, project) &&
log.isLoggable(Level.WARNING)) {
log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", bucket);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
Expand All @@ -41,6 +42,8 @@
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;
import java.util.logging.Logger;

import com.google.common.base.Strings;
import org.threeten.bp.Duration;

/**
Expand Down Expand Up @@ -121,8 +124,29 @@ public void run() {
*/
public static Boolean forceDelete(Storage storage, String bucket, long timeout, TimeUnit unit)
throws InterruptedException, ExecutionException {
return forceDelete(storage, bucket, timeout, unit, "");
}

/**
* Deletes a bucket, even if non-empty. Objects in the bucket are listed and deleted until bucket
* deletion succeeds or {@code timeout} expires. To allow for the timeout, this method uses a
* separate thread to send the delete requests. Use
* {@link #forceDelete(Storage storage, String bucket)} if spawning an additional thread is
* undesirable, such as in the App Engine production runtime.
*
* @param storage the storage service to be used to issue requests
* @param bucket the bucket to be deleted
* @param timeout the maximum time to wait
* @param unit the time unit of the timeout argument
* @param userProject the project to bill for requester-pays buckets (or "")
* @return true if deletion succeeded, false if timeout expired
* @throws InterruptedException if the thread deleting the bucket is interrupted while waiting
* @throws ExecutionException if an exception was thrown while deleting bucket or bucket objects
*/
public static Boolean forceDelete(Storage storage, String bucket, long timeout, TimeUnit unit, String userProject)
throws InterruptedException, ExecutionException {
ExecutorService executor = Executors.newSingleThreadExecutor();
Future<Boolean> future = executor.submit(new DeleteBucketTask(storage, bucket));
Future<Boolean> future = executor.submit(new DeleteBucketTask(storage, bucket, userProject));
try {
return future.get(timeout, unit);
} catch (TimeoutException ex) {
Expand Down Expand Up @@ -210,26 +234,52 @@ private static RetrySettings retrySettings() {

private static class DeleteBucketTask implements Callable<Boolean> {

private Storage storage;
private String bucket;
private final Storage storage;
private final String bucket;
private final String userProject;

public DeleteBucketTask(Storage storage, String bucket) {
this.storage = storage;
this.bucket = bucket;
this.userProject = "";
}

public DeleteBucketTask(Storage storage, String bucket, String userProject) {
this.storage = storage;
this.bucket = bucket;
this.userProject = userProject;
}

@Override
public Boolean call() {
while (true) {
ArrayList<BlobId> ids = new ArrayList<>();
for (BlobInfo info : storage.list(bucket, BlobListOption.versions(true)).getValues()) {
Page<Blob> listedBlobs;
if (Strings.isNullOrEmpty(userProject)) {
listedBlobs = storage.list(bucket, BlobListOption.versions(true));
} else {
listedBlobs = storage.list(bucket, BlobListOption.versions(true), BlobListOption.userProject(userProject));
}
for (BlobInfo info : listedBlobs.getValues()) {
ids.add(info.getBlobId());
}
if (!ids.isEmpty()) {
storage.delete(ids);
List<Boolean> results = storage.delete(ids);
if (!Strings.isNullOrEmpty(userProject)) {
for (int i=0; i<results.size(); i++) {
if (!results.get(i)) {
// deleting that blob failed. Let's try in a different way.
storage.delete(bucket, ids.get(i).getName(), Storage.BlobSourceOption.userProject(userProject));
}
}
}
}
try {
storage.delete(bucket);
if (Strings.isNullOrEmpty(userProject)) {
storage.delete(bucket);
} else {
storage.delete(bucket, Storage.BucketSourceOption.userProject(userProject));
}
return true;
} catch (StorageException e) {
if (e.getCode() == 409) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ public class RemoteStorageHelperTest {
private static final InputStream JSON_KEY_STREAM = new ByteArrayInputStream(JSON_KEY.getBytes());
private static final StorageException RETRYABLE_EXCEPTION = new StorageException(409, "");
private static final StorageException FATAL_EXCEPTION = new StorageException(500, "");
private static final String BLOB_NAME2 ="n2";
private static final BlobId BLOB_ID1 = BlobId.of(BUCKET_NAME, "n1");
private static final BlobId BLOB_ID2 = BlobId.of(BUCKET_NAME, "n2");
private static final BlobId BLOB_ID2 = BlobId.of(BUCKET_NAME, BLOB_NAME2);

private Blob blob1;
private Blob blob2;
Expand Down Expand Up @@ -221,6 +222,29 @@ public void testForceDeleteNoTimeoutFail() {
}
}

@Test
public void testForceDeleteRetriesWithUserProject() throws Exception {
final String USER_PROJECT="user-project";

This comment was marked as spam.

Storage storageMock = EasyMock.createMock(Storage.class);
EasyMock.expect(blob1.getBlobId()).andReturn(BLOB_ID1);
EasyMock.expect(blob2.getBlobId()).andReturn(BLOB_ID2);
EasyMock.expect(blob2.getName()).andReturn(BLOB_NAME2);
ArrayList<BlobId> ids = new ArrayList<>();
ids.add(BLOB_ID1);
ids.add(BLOB_ID2);
EasyMock.expect(storageMock.delete(ids)).andReturn(ImmutableList.of(Boolean.TRUE, Boolean.FALSE)).anyTimes();
EasyMock.expect(storageMock.delete(BUCKET_NAME, BLOB_NAME2, Storage.BlobSourceOption.userProject(USER_PROJECT))).andReturn(true).anyTimes();
EasyMock.expect(storageMock.list(BUCKET_NAME, BlobListOption.versions(true), BlobListOption.userProject(USER_PROJECT)))
.andReturn(blobPage);
EasyMock.expect(storageMock.delete(BUCKET_NAME, Storage.BucketSourceOption.userProject(USER_PROJECT))).andReturn(true);
EasyMock.replay(storageMock, blob1, blob2);
try {
RemoteStorageHelper.forceDelete(storageMock, BUCKET_NAME, 5, TimeUnit.SECONDS, USER_PROJECT);
} finally {
EasyMock.verify(storageMock);
}
}

@Test
public void testCreateFromStream() {
RemoteStorageHelper helper = RemoteStorageHelper.create(PROJECT_ID, JSON_KEY_STREAM);
Expand Down