Skip to content

Commit 0ce262c

Browse files
Fix Bug in Azure Repo Exception Handling (#47968) (#48031)
We were incorrectly handling `IOExceptions` thrown by the `InputStream` side of the upload operation, resulting in a `ClassCastException` as we expected to never get `IOException` from the Azure SDK code but we do in practice. This PR also sets an assertion on `markSupported` for the streams used by the SDK as adding the test for this scenario revealed that the SDK client would retry uploads for non-mark-supporting streams on `IOException` in the `InputStream`.
1 parent 653e0be commit 0ce262c

File tree

4 files changed

+19
-20
lines changed

4 files changed

+19
-20
lines changed

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ private boolean blobExists(String blobName) {
6262
logger.trace("blobExists({})", blobName);
6363
try {
6464
return blobStore.blobExists(buildKey(blobName));
65-
} catch (URISyntaxException | StorageException e) {
65+
} catch (URISyntaxException | StorageException | IOException e) {
6666
logger.warn("can not access [{}] in container {{}}: {}", blobName, blobStore, e.getMessage());
6767
}
6868
return false;
@@ -97,7 +97,6 @@ public InputStream readBlob(String blobName) throws IOException {
9797
@Override
9898
public void writeBlob(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists) throws IOException {
9999
logger.trace("writeBlob({}, stream, {})", buildKey(blobName), blobSize);
100-
101100
try {
102101
blobStore.writeBlob(buildKey(blobName), inputStream, blobSize, failIfAlreadyExists);
103102
} catch (URISyntaxException|StorageException e) {

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import java.io.IOException;
3434
import java.io.InputStream;
3535
import java.net.URISyntaxException;
36-
import java.nio.file.FileAlreadyExistsException;
3736
import java.util.Collections;
3837
import java.util.Map;
3938
import java.util.concurrent.Executor;
@@ -84,11 +83,11 @@ public BlobContainer blobContainer(BlobPath path) {
8483
public void close() {
8584
}
8685

87-
public boolean blobExists(String blob) throws URISyntaxException, StorageException {
86+
public boolean blobExists(String blob) throws URISyntaxException, StorageException, IOException {
8887
return service.blobExists(clientName, container, blob);
8988
}
9089

91-
public void deleteBlob(String blob) throws URISyntaxException, StorageException {
90+
public void deleteBlob(String blob) throws URISyntaxException, StorageException, IOException {
9291
service.deleteBlob(clientName, container, blob);
9392
}
9493

@@ -102,17 +101,17 @@ public InputStream getInputStream(String blob) throws URISyntaxException, Storag
102101
}
103102

104103
public Map<String, BlobMetaData> listBlobsByPrefix(String keyPath, String prefix)
105-
throws URISyntaxException, StorageException {
104+
throws URISyntaxException, StorageException, IOException {
106105
return service.listBlobsByPrefix(clientName, container, keyPath, prefix);
107106
}
108107

109-
public Map<String, BlobContainer> children(BlobPath path) throws URISyntaxException, StorageException {
108+
public Map<String, BlobContainer> children(BlobPath path) throws URISyntaxException, StorageException, IOException {
110109
return Collections.unmodifiableMap(service.children(clientName, container, path).stream().collect(
111110
Collectors.toMap(Function.identity(), name -> new AzureBlobContainer(path.add(name), this, threadPool))));
112111
}
113112

114113
public void writeBlob(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists)
115-
throws URISyntaxException, StorageException, FileAlreadyExistsException {
114+
throws URISyntaxException, StorageException, IOException {
116115
service.writeBlob(this.clientName, container, blobName, inputStream, blobSize, failIfAlreadyExists);
117116
}
118117
}

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ public InputStream getInputStream(String account, String container, String blob)
262262
}
263263

264264
public Map<String, BlobMetaData> listBlobsByPrefix(String account, String container, String keyPath, String prefix)
265-
throws URISyntaxException, StorageException {
265+
throws URISyntaxException, StorageException, IOException {
266266
// NOTE: this should be here: if (prefix == null) prefix = "";
267267
// however, this is really inefficient since deleteBlobsByPrefix enumerates everything and
268268
// then does a prefix match on the result; it should just call listBlobsByPrefix with the prefix!
@@ -290,7 +290,7 @@ public Map<String, BlobMetaData> listBlobsByPrefix(String account, String contai
290290
return blobsBuilder.immutableMap();
291291
}
292292

293-
public Set<String> children(String account, String container, BlobPath path) throws URISyntaxException, StorageException {
293+
public Set<String> children(String account, String container, BlobPath path) throws URISyntaxException, StorageException, IOException {
294294
final Set<String> blobsBuilder = new HashSet<>();
295295
final Tuple<CloudBlobClient, Supplier<OperationContext>> client = client(account);
296296
final CloudBlobContainer blobContainer = client.v1().getContainerReference(container);
@@ -314,8 +314,9 @@ public Set<String> children(String account, String container, BlobPath path) thr
314314
}
315315

316316
public void writeBlob(String account, String container, String blobName, InputStream inputStream, long blobSize,
317-
boolean failIfAlreadyExists)
318-
throws URISyntaxException, StorageException, FileAlreadyExistsException {
317+
boolean failIfAlreadyExists) throws URISyntaxException, StorageException, IOException {
318+
assert inputStream.markSupported()
319+
: "Should not be used with non-mark supporting streams as their retry handling in the SDK is broken";
319320
logger.trace(() -> new ParameterizedMessage("writeBlob({}, stream, {})", blobName, blobSize));
320321
final Tuple<CloudBlobClient, Supplier<OperationContext>> client = client(account);
321322
final CloudBlobContainer blobContainer = client.v1().getContainerReference(container);

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/SocketAccess.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.repositories.azure;
2121

2222
import com.microsoft.azure.storage.StorageException;
23+
import org.apache.logging.log4j.core.util.Throwables;
2324
import org.elasticsearch.SpecialPermission;
2425

2526
import java.io.IOException;
@@ -44,7 +45,9 @@ public static <T> T doPrivilegedIOException(PrivilegedExceptionAction<T> operati
4445
try {
4546
return AccessController.doPrivileged(operation);
4647
} catch (PrivilegedActionException e) {
47-
throw (IOException) e.getCause();
48+
Throwables.rethrow(e.getCause());
49+
assert false : "always throws";
50+
return null;
4851
}
4952
}
5053

@@ -53,7 +56,9 @@ public static <T> T doPrivilegedException(PrivilegedExceptionAction<T> operation
5356
try {
5457
return AccessController.doPrivileged(operation);
5558
} catch (PrivilegedActionException e) {
56-
throw (StorageException) e.getCause();
59+
Throwables.rethrow(e.getCause());
60+
assert false : "always throws";
61+
return null;
5762
}
5863
}
5964

@@ -65,12 +70,7 @@ public static void doPrivilegedVoidException(StorageRunnable action) throws Stor
6570
return null;
6671
});
6772
} catch (PrivilegedActionException e) {
68-
Throwable cause = e.getCause();
69-
if (cause instanceof StorageException) {
70-
throw (StorageException) cause;
71-
} else {
72-
throw (URISyntaxException) cause;
73-
}
73+
Throwables.rethrow(e.getCause());
7474
}
7575
}
7676

0 commit comments

Comments
 (0)