From 92382d862df212c1e1106264f6588264f08e6367 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Wed, 25 May 2016 16:30:48 +0200 Subject: [PATCH] Fix remove of azure files Probably when we updated Azure SDK, we introduced a regression. Actually, we are not able to remove files anymore. For example, if you register a new azure repository, the snapshot service tries to create a temp file and then remove it. Removing does not work and you can see in logs: ``` [2016-05-18 11:03:24,914][WARN ][org.elasticsearch.cloud.azure.blobstore] [azure] can not remove [tests-ilmRPJ8URU-sh18yj38O6g/] in container {elasticsearch-snapshots}: The specified blob does not exist. ``` This fix deals with that. It now list all the files in a flatten mode, remove in the full URL the server and the container name. As an example, when you are removing a blob which full name is `https://dpi24329.blob.core.windows.net/elasticsearch-snapshots/bar/test` you need to actually call Azure SDK with `bar/test` as the path, `elasticsearch-snapshots` is the container. Related to #16472. Related to #18436. Backport of #18451 in 2.x branch To test it, I ran some manual tests: On my laptop, create a file `/path/to/azure/config/elasticsearch.yml`: ```yml cloud.azure.storage.default.account: ACCOUNT cloud.azure.storage.default.key: KEY ``` Run `AzureRepositoryF#main()` with `-Des.cluster.routing.allocation.disk.threshold_enabled=false -Des.path.home=/path/to/azure/` options. Then run: ```sh curl -XDELETE localhost:9200/foo?pretty curl -XDELETE localhost:9200/_snapshot/my_backup1?pretty curl -XPUT localhost:9200/foo/bar/1?pretty -d '{ "foo": "bar" }' curl -XPOST localhost:9200/foo/_refresh?pretty curl -XGET localhost:9200/foo/_count?pretty curl -XPUT localhost:9200/_snapshot/my_backup1?pretty -d '{ "type": "azure" }' curl -XPOST "localhost:9200/_snapshot/my_backup1/snap1?pretty&wait_for_completion=true" curl -XDELETE localhost:9200/foo?pretty curl -XPOST "localhost:9200/_snapshot/my_backup1/snap1/_restore?pretty&wait_for_completion=true" curl -XGET localhost:9200/foo/_count?pretty ``` Then check files we have on azure platform using the console. Then run: ``` curl -XDELETE localhost:9200/_snapshot/my_backup1/snap1?pretty ``` Then check files we have on azure platform using the console and verify that everything has been cleaned. --- .../azure/blobstore/AzureBlobContainer.java | 7 ++ .../storage/AzureStorageServiceImpl.java | 64 +++++++++++++------ .../storage/AzureStorageServiceTest.java | 13 ++++ 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java b/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java index db1b5c7d3d6ab..2862da31c68d2 100644 --- a/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java +++ b/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobContainer.java @@ -60,6 +60,7 @@ public AzureBlobContainer(String repositoryName, BlobPath path, AzureBlobStore b @Override public boolean blobExists(String blobName) { + logger.trace("blobExists({})", blobName); try { return blobStore.blobExists(blobStore.container(), buildKey(blobName)); } catch (URISyntaxException | StorageException e) { @@ -70,6 +71,7 @@ public boolean blobExists(String blobName) { @Override public InputStream openInput(String blobName) throws IOException { + logger.trace("openInput({})", blobName); try { return blobStore.getInputStream(blobStore.container(), buildKey(blobName)); } catch (StorageException e) { @@ -84,6 +86,7 @@ public InputStream openInput(String blobName) throws IOException { @Override public OutputStream createOutput(String blobName) throws IOException { + logger.trace("createOutput({})", blobName); try { return new AzureOutputStream(blobStore.getOutputStream(blobStore.container(), buildKey(blobName))); } catch (StorageException e) { @@ -100,6 +103,7 @@ public OutputStream createOutput(String blobName) throws IOException { @Override public void deleteBlob(String blobName) throws IOException { + logger.trace("deleteBlob({})", blobName); try { blobStore.deleteBlob(blobStore.container(), buildKey(blobName)); } catch (URISyntaxException | StorageException e) { @@ -110,6 +114,7 @@ public void deleteBlob(String blobName) throws IOException { @Override public Map listBlobsByPrefix(@Nullable String prefix) throws IOException { + logger.trace("listBlobsByPrefix({})", prefix); try { return blobStore.listBlobsByPrefix(blobStore.container(), keyPath, prefix); @@ -121,6 +126,7 @@ public Map listBlobsByPrefix(@Nullable String prefix) thro @Override public void move(String sourceBlobName, String targetBlobName) throws IOException { + logger.trace("move({}, {})", sourceBlobName, targetBlobName); try { String source = keyPath + sourceBlobName; String target = keyPath + targetBlobName; @@ -139,6 +145,7 @@ public void move(String sourceBlobName, String targetBlobName) throws IOExceptio @Override public Map listBlobs() throws IOException { + logger.trace("listBlobs()"); return listBlobsByPrefix(null); } diff --git a/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java b/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java index a3cc1f0c67a4b..472c0ced6fa97 100644 --- a/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java +++ b/plugins/cloud-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceImpl.java @@ -133,8 +133,8 @@ CloudBlobClient getSelectedClient(String account, LocationMode mode) { public boolean doesContainerExist(String account, LocationMode mode, String container) { try { CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); - return blob_container.exists(); + CloudBlobContainer blobContainer = client.getContainerReference(container); + return blobContainer.exists(); } catch (Exception e) { logger.error("can not access container [{}]", container); } @@ -144,25 +144,25 @@ public boolean doesContainerExist(String account, LocationMode mode, String cont @Override public void removeContainer(String account, LocationMode mode, String container) throws URISyntaxException, StorageException { CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); + CloudBlobContainer blobContainer = client.getContainerReference(container); // TODO Should we set some timeout and retry options? /* BlobRequestOptions options = new BlobRequestOptions(); options.setTimeoutIntervalInMs(1000); options.setRetryPolicyFactory(new RetryNoRetry()); - blob_container.deleteIfExists(options, null); + blobContainer.deleteIfExists(options, null); */ logger.trace("removing container [{}]", container); - blob_container.deleteIfExists(); + blobContainer.deleteIfExists(); } @Override public void createContainer(String account, LocationMode mode, String container) throws URISyntaxException, StorageException { try { CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); + CloudBlobContainer blobContainer = client.getContainerReference(container); logger.trace("creating container [{}]", container); - blob_container.createIfNotExists(); + blobContainer.createIfNotExists(); } catch (IllegalArgumentException e) { logger.trace("fails creating container [{}]", container, e.getMessage()); throw new RepositoryException(container, e.getMessage()); @@ -175,22 +175,44 @@ public void deleteFiles(String account, LocationMode mode, String container, Str // Container name must be lower case. CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); - if (blob_container.exists()) { - for (ListBlobItem blobItem : blob_container.listBlobs(path)) { - logger.trace("removing blob [{}]", blobItem.getUri()); - deleteBlob(account, mode, container, blobItem.getUri().toString()); + CloudBlobContainer blobContainer = client.getContainerReference(container); + if (blobContainer.exists()) { + // We list the blobs using a flat blob listing mode + for (ListBlobItem blobItem : blobContainer.listBlobs(path, true)) { + String blobName = blobNameFromUri(blobItem.getUri()); + logger.trace("removing blob [{}] full URI was [{}]", blobName, blobItem.getUri()); + deleteBlob(account, mode, container, blobName); } } } + /** + * Extract the blob name from a URI like https://myservice.azure.net/container/path/to/myfile + * It should remove the container part (first part of the path) and gives path/to/myfile + * @param uri URI to parse + * @return The blob name relative to the container + */ + public static String blobNameFromUri(URI uri) { + String path = uri.getPath(); + + // We remove the container name from the path + // The 3 magic number cames from the fact if path is /container/path/to/myfile + // First occurrence is empty "/" + // Second occurrence is "container + // Last part contains "path/to/myfile" which is what we want to get + String[] splits = path.split("/", 3); + + // We return the remaining end of the string + return splits[2]; + } + @Override public boolean blobExists(String account, LocationMode mode, String container, String blob) throws URISyntaxException, StorageException { // Container name must be lower case. CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); - if (blob_container.exists()) { - CloudBlockBlob azureBlob = blob_container.getBlockBlobReference(blob); + CloudBlobContainer blobContainer = client.getContainerReference(container); + if (blobContainer.exists()) { + CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blob); return azureBlob.exists(); } @@ -203,10 +225,10 @@ public void deleteBlob(String account, LocationMode mode, String container, Stri // Container name must be lower case. CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); - if (blob_container.exists()) { + CloudBlobContainer blobContainer = client.getContainerReference(container); + if (blobContainer.exists()) { logger.trace("container [{}]: blob [{}] found. removing.", container, blob); - CloudBlockBlob azureBlob = blob_container.getBlockBlobReference(blob); + CloudBlockBlob azureBlob = blobContainer.getBlockBlobReference(blob); azureBlob.delete(); } } @@ -266,10 +288,10 @@ public void moveBlob(String account, LocationMode mode, String container, String logger.debug("moveBlob container [{}], sourceBlob [{}], targetBlob [{}]", container, sourceBlob, targetBlob); CloudBlobClient client = this.getSelectedClient(account, mode); - CloudBlobContainer blob_container = client.getContainerReference(container); - CloudBlockBlob blobSource = blob_container.getBlockBlobReference(sourceBlob); + CloudBlobContainer blobContainer = client.getContainerReference(container); + CloudBlockBlob blobSource = blobContainer.getBlockBlobReference(sourceBlob); if (blobSource.exists()) { - CloudBlockBlob blobTarget = blob_container.getBlockBlobReference(targetBlob); + CloudBlockBlob blobTarget = blobContainer.getBlockBlobReference(targetBlob); blobTarget.startCopy(blobSource); blobSource.delete(); logger.debug("moveBlob container [{}], sourceBlob [{}], targetBlob [{}] -> done", container, sourceBlob, targetBlob); diff --git a/plugins/cloud-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceTest.java b/plugins/cloud-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceTest.java index f8e8f593411d4..d1471c72e11ce 100644 --- a/plugins/cloud-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceTest.java +++ b/plugins/cloud-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceTest.java @@ -25,7 +25,9 @@ import org.elasticsearch.test.ESTestCase; import java.net.URI; +import java.net.URISyntaxException; +import static org.elasticsearch.cloud.azure.storage.AzureStorageServiceImpl.blobNameFromUri; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; @@ -168,4 +170,15 @@ void createClient(AzureStorageSettings azureStorageSettings) { new CloudBlobClient(URI.create("https://" + azureStorageSettings.getName()))); } } + + public void testBlobNameFromUri() throws URISyntaxException { + String name = blobNameFromUri(new URI("https://myservice.azure.net/container/path/to/myfile")); + assertThat(name, is("path/to/myfile")); + name = blobNameFromUri(new URI("http://myservice.azure.net/container/path/to/myfile")); + assertThat(name, is("path/to/myfile")); + name = blobNameFromUri(new URI("http://127.0.0.1/container/path/to/myfile")); + assertThat(name, is("path/to/myfile")); + name = blobNameFromUri(new URI("https://127.0.0.1/container/path/to/myfile")); + assertThat(name, is("path/to/myfile")); + } }