Skip to content

Commit

Permalink
Do not check for Azure container existence (#31617)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tlrx
tlrx authored Jun 28, 2018
1 parent 9d523d0 commit 0f0d13e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 47 deletions.
12 changes: 6 additions & 6 deletions ...pository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
63 changes: 27 additions & 36 deletions ...ory-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,17 @@ public void createContainer(String account, String container) throws URISyntaxEx
public void deleteFiles(String account, String container, String path) throws URISyntaxException, StorageException {
final Tuple<CloudBlobClient, Supplier<OperationContext>> 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());
}
});
}
Expand Down Expand Up @@ -192,11 +190,8 @@ public boolean blobExists(String account, String container, String blob)
final Tuple<CloudBlobClient, Supplier<OperationContext>> 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());
});
}

Expand All @@ -207,11 +202,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());
});
}

Expand All @@ -238,19 +231,17 @@ public Map<String, BlobMetaData> 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();
Expand All @@ -264,8 +255,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())) {
Expand Down
13 changes: 8 additions & 5 deletions ...ory-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, BlobMetaData> blobs = listBlobsByPrefix(account, container, path, null);
blobs.keySet().forEach(key -> deleteBlob(account, container, key));
for (String key : blobs.keySet()) {
deleteBlob(account, container, key);
}
}

@Override
Expand All @@ -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
Expand Down

0 comments on commit 0f0d13e

Please sign in to comment.