From 4020e4e0f6c740f3d7810a60c0514e3be94df3cb Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 28 Jun 2018 10:57:49 +0200 Subject: [PATCH] Do not check for Azure container existence (#31617) The current AzureStorageServiceImpl always checks if the Azure container exists before reading or writing an object to the Azure container. This commit removes this behavior, reducing the number of overhall requests executed for all snapshots operations. --- .../azure/blobstore/AzureBlobContainer.java | 12 ++-- .../storage/AzureStorageServiceImpl.java | 63 ++++++++----------- .../storage/AzureStorageServiceMock.java | 13 ++-- 3 files changed, 41 insertions(+), 47 deletions(-) diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java index 36b56a114a729..686926c3a991e 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java @@ -100,14 +100,14 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t public void deleteBlob(String blobName) throws IOException { logger.trace("deleteBlob({})", blobName); - if (!blobExists(blobName)) { - throw new NoSuchFileException("Blob [" + blobName + "] does not exist"); - } - try { blobStore.deleteBlob(buildKey(blobName)); - } catch (URISyntaxException | StorageException e) { - logger.warn("can not access [{}] in container {{}}: {}", blobName, blobStore, e.getMessage()); + } catch (StorageException e) { + if (e.getHttpStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) { + throw new NoSuchFileException(e.getMessage()); + } + throw new IOException(e); + } catch (URISyntaxException e) { throw new IOException(e); } } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java index 7ea4dcca5fcbb..d4e260194b599 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java @@ -158,19 +158,17 @@ public void createContainer(String account, String container) throws URISyntaxEx public void deleteFiles(String account, String container, String path) throws URISyntaxException, StorageException { final Tuple> client = client(account); // container name must be lower case. - final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); logger.trace(() -> new ParameterizedMessage("delete files container [{}], path [{}]", container, path)); SocketAccess.doPrivilegedVoidException(() -> { - if (blobContainer.exists()) { - // list the blobs using a flat blob listing mode - for (final ListBlobItem blobItem : blobContainer.listBlobs(path, true, EnumSet.noneOf(BlobListingDetails.class), null, - client.v2().get())) { - final String blobName = blobNameFromUri(blobItem.getUri()); - logger.trace(() -> new ParameterizedMessage("removing blob [{}] full URI was [{}]", blobName, blobItem.getUri())); - // don't call {@code #deleteBlob}, use the same client - final CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blobName); - azureBlob.delete(DeleteSnapshotsOption.NONE, null, null, client.v2().get()); - } + // list the blobs using a flat blob listing mode + final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); + for (final ListBlobItem blobItem : blobContainer.listBlobs(path, true, EnumSet.noneOf(BlobListingDetails.class), null, + client.v2().get())) { + final String blobName = blobNameFromUri(blobItem.getUri()); + logger.trace(() -> new ParameterizedMessage("removing blob [{}] full URI was [{}]", blobName, blobItem.getUri())); + // don't call {@code #deleteBlob}, use the same client + final CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blobName); + azureBlob.delete(DeleteSnapshotsOption.NONE, null, null, client.v2().get()); } }); } @@ -200,11 +198,8 @@ public boolean blobExists(String account, String container, String blob) final Tuple> client = client(account); final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); return SocketAccess.doPrivilegedException(() -> { - if (blobContainer.exists(null, null, client.v2().get())) { - final CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blob); - return azureBlob.exists(null, null, client.v2().get()); - } - return false; + final CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blob); + return azureBlob.exists(null, null, client.v2().get()); }); } @@ -215,11 +210,9 @@ public void deleteBlob(String account, String container, String blob) throws URI final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); logger.trace(() -> new ParameterizedMessage("delete blob for container [{}], blob [{}]", container, blob)); SocketAccess.doPrivilegedVoidException(() -> { - if (blobContainer.exists(null, null, client.v2().get())) { - final CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blob); - logger.trace(() -> new ParameterizedMessage("container [{}]: blob [{}] found. removing.", container, blob)); - azureBlob.delete(DeleteSnapshotsOption.NONE, null, null, client.v2().get()); - } + final CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blob); + logger.trace(() -> new ParameterizedMessage("container [{}]: blob [{}] found. removing.", container, blob)); + azureBlob.delete(DeleteSnapshotsOption.NONE, null, null, client.v2().get()); }); } @@ -245,19 +238,17 @@ public Map listBlobsByPrefix(String account, String contai final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); logger.trace(() -> new ParameterizedMessage("listing container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix)); SocketAccess.doPrivilegedVoidException(() -> { - if (blobContainer.exists()) { - for (final ListBlobItem blobItem : blobContainer.listBlobs(keyPath + (prefix == null ? "" : prefix), false, - enumBlobListingDetails, null, client.v2().get())) { - final URI uri = blobItem.getUri(); - logger.trace(() -> new ParameterizedMessage("blob url [{}]", uri)); - // uri.getPath is of the form /container/keyPath.* and we want to strip off the /container/ - // this requires 1 + container.length() + 1, with each 1 corresponding to one of the / - final String blobPath = uri.getPath().substring(1 + container.length() + 1); - final BlobProperties properties = ((CloudBlockBlob) blobItem).getProperties(); - final String name = blobPath.substring(keyPath.length()); - logger.trace(() -> new ParameterizedMessage("blob url [{}], name [{}], size [{}]", uri, name, properties.getLength())); - blobsBuilder.put(name, new PlainBlobMetaData(name, properties.getLength())); - } + for (final ListBlobItem blobItem : blobContainer.listBlobs(keyPath + (prefix == null ? "" : prefix), false, + enumBlobListingDetails, null, client.v2().get())) { + final URI uri = blobItem.getUri(); + logger.trace(() -> new ParameterizedMessage("blob url [{}]", uri)); + // uri.getPath is of the form /container/keyPath.* and we want to strip off the /container/ + // this requires 1 + container.length() + 1, with each 1 corresponding to one of the / + final String blobPath = uri.getPath().substring(1 + container.length() + 1); + final BlobProperties properties = ((CloudBlockBlob) blobItem).getProperties(); + final String name = blobPath.substring(keyPath.length()); + logger.trace(() -> new ParameterizedMessage("blob url [{}], name [{}], size [{}]", uri, name, properties.getLength())); + blobsBuilder.put(name, new PlainBlobMetaData(name, properties.getLength())); } }); return blobsBuilder.immutableMap(); @@ -271,8 +262,8 @@ public void writeBlob(String account, String container, String blobName, InputSt final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); final CloudBlockBlob blob = blobContainer.getBlockBlobReference(blobName); try { - SocketAccess.doPrivilegedVoidException(() -> blob.upload(inputStream, blobSize, AccessCondition.generateIfNotExistsCondition(), - null, client.v2().get())); + SocketAccess.doPrivilegedVoidException(() -> + blob.upload(inputStream, blobSize, AccessCondition.generateIfNotExistsCondition(), null, client.v2().get())); } catch (final StorageException se) { if (se.getHttpStatusCode() == HttpURLConnection.HTTP_CONFLICT && StorageErrorCodeStrings.BLOB_ALREADY_EXISTS.equals(se.getErrorCode())) { diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceMock.java b/plugins/repository-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceMock.java index dc666bfce3e8d..72bba87e5201e 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceMock.java +++ b/plugins/repository-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceMock.java @@ -22,7 +22,6 @@ import com.microsoft.azure.storage.OperationContext; import com.microsoft.azure.storage.StorageException; import com.microsoft.azure.storage.blob.CloudBlobClient; - import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.common.collect.MapBuilder; @@ -72,9 +71,11 @@ public void createContainer(String account, String container) { } @Override - public void deleteFiles(String account, String container, String path) { + public void deleteFiles(String account, String container, String path) throws URISyntaxException, StorageException { final Map blobs = listBlobsByPrefix(account, container, path, null); - blobs.keySet().forEach(key -> deleteBlob(account, container, key)); + for (String key : blobs.keySet()) { + deleteBlob(account, container, key); + } } @Override @@ -83,8 +84,10 @@ public boolean blobExists(String account, String container, String blob) { } @Override - public void deleteBlob(String account, String container, String blob) { - blobs.remove(blob); + public void deleteBlob(String account, String container, String blob) throws URISyntaxException, StorageException { + if (blobs.remove(blob) == null) { + throw new StorageException("BlobNotFound", "[" + blob + "] does not exist.", 404, null, null); + } } @Override