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.

Sign up for GitHub

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

Implement Bulk Deletes for GCS Repository #41368

Merged
merged 12 commits into from
Apr 30, 2019
Merged

Implement Bulk Deletes for GCS Repository #41368

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bc18a17
Implement Bulk Deletes for GCS Repository
original-brownbear Apr 19, 2019
b971752
CR: no need for manual batching
original-brownbear Apr 23, 2019
1ae7460
Merge remote-tracking branch 'elastic/master' into gcs-batch-delete
original-brownbear Apr 23, 2019
ae58d9d
Merge remote-tracking branch 'elastic/master' into gcs-batch-delete
original-brownbear Apr 26, 2019
a029e7b
CR: use low level delete to not have to stat blobs
original-brownbear Apr 26, 2019
4199c2a
Merge remote-tracking branch 'elastic/master' into gcs-batch-delete
original-brownbear Apr 29, 2019
77e5875
CR: rename method and better mock
original-brownbear Apr 29, 2019
d5a05fa
CR: collect exceptions
original-brownbear Apr 29, 2019
729e8b9
Merge remote-tracking branch 'elastic/master' into gcs-batch-delete
original-brownbear Apr 29, 2019
1d8d9c0
CR: throw IOException instead of StorageException
original-brownbear Apr 29, 2019
2838d3d
Merge remote-tracking branch 'elastic/master' into gcs-batch-delete
original-brownbear Apr 30, 2019
a0e616c
CR: collect list of failed blob names in ex message
original-brownbear Apr 30, 2019
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
9 changes: 8 additions & 1 deletion ...gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

class GoogleCloudStorageBlobContainer extends AbstractBlobContainer {

Expand Down Expand Up @@ -78,7 +80,12 @@ public void deleteBlob(String blobName) throws IOException {
blobStore.deleteBlob(buildKey(blobName));
}

protected String buildKey(String blobName) {
@Override
public void deleteBlobsIgnoringIfNotExists(List<String> blobNames) throws IOException {
blobStore.deleteBlobs(blobNames.stream().map(this::buildKey).collect(Collectors.toList()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method on blobstore should now also be renamed to deleteBlobsIgnoringIfNotExists

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

private String buildKey(String blobName) {
assert blobName != null;
return path + blobName;
}
Expand Down
20 changes: 16 additions & 4 deletions ...ory-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobMetaData;
Expand Down Expand Up @@ -105,7 +106,7 @@ public void close() {
* @param bucketName name of the bucket
* @return true iff the bucket exists
*/
boolean doesBucketExist(String bucketName) {
private boolean doesBucketExist(String bucketName) {
try {
final Bucket bucket = SocketAccess.doPrivilegedIOException(() -> client().get(bucketName));
return bucket != null;
Expand Down Expand Up @@ -295,7 +296,7 @@ void deleteBlob(String blobName) throws IOException {
*
* @param prefix prefix of the blobs to delete
*/
void deleteBlobsByPrefix(String prefix) throws IOException {
private void deleteBlobsByPrefix(String prefix) throws IOException {
deleteBlobs(listBlobsByPrefix("", prefix).keySet());
}

Expand All @@ -319,8 +320,19 @@ void deleteBlobs(Collection<String> blobNames) throws IOException {
boolean failed = false;
for (int i = 0; i < blobIdsToDelete.size(); i++) {
if (deletedStatuses.get(i) == false) {
logger.error("Failed to delete blob [{}] in bucket [{}]", blobIdsToDelete.get(i).getName(), bucketName);
failed = true;
final BlobId blobId = blobIdsToDelete.get(i);
try {
Blob blob = SocketAccess.doPrivilegedIOException(() -> client().get(blobId));
andrershov marked this conversation as resolved.
Show resolved Hide resolved
if (blob != null) {
logger.error("Failed to delete blob [{}] in bucket [{}]", blobId.getName(), bucketName);
original-brownbear marked this conversation as resolved.
Show resolved Hide resolved
failed = true;
}
} catch (Exception e) {
logger.error(
original-brownbear marked this conversation as resolved.
Show resolved Hide resolved
new ParameterizedMessage(
"Failed to delete and then stat blob [{}] in bucket [{}]", blobId.getName(), bucketName), e);
failed = true;
}
}
}
if (failed) {
Expand Down