[plugin] repository-azure is not working properly hangs on basic operations (#1740) (#1749)

This commit fixes repository-azure hanging on basic operations. This will be reverted 
once it's fixed upstream in the Azure library.

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
This commit is contained in:
Andriy Redko 2021-12-16 16:10:52 -05:00 committed by GitHub
parent ef44182731
commit 20692a2ff2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 140 additions and 59 deletions

View File

@ -283,4 +283,23 @@ task azureThirdPartyTest(type: Test) {
nonInputProperties.systemProperty 'test.azure.endpoint_suffix', "${-> azureAddress.call() }"
}
}
check.dependsOn(azureThirdPartyTest)
task azureThirdPartyDefaultXmlTest(type: Test) {
SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
SourceSet internalTestSourceSet = sourceSets.getByName(InternalClusterTestPlugin.SOURCE_SET_NAME)
setTestClassesDirs(internalTestSourceSet.getOutput().getClassesDirs())
setClasspath(internalTestSourceSet.getRuntimeClasspath())
dependsOn tasks.internalClusterTest
include '**/AzureStorageCleanupThirdPartyTests.class'
systemProperty 'javax.xml.stream.XMLInputFactory', "com.sun.xml.internal.stream.XMLInputFactoryImpl"
systemProperty 'test.azure.account', azureAccount ? azureAccount : ""
systemProperty 'test.azure.key', azureKey ? azureKey : ""
systemProperty 'test.azure.sas_token', azureSasToken ? azureSasToken : ""
systemProperty 'test.azure.container', azureContainer ? azureContainer : ""
systemProperty 'test.azure.base', (azureBasePath ? azureBasePath : "") + "_third_party_tests_" + BuildParams.testSeed
if (useFixture) {
nonInputProperties.systemProperty 'test.azure.endpoint_suffix', "${-> azureAddress.call() }"
}
}
check.dependsOn(azureThirdPartyTest, azureThirdPartyDefaultXmlTest)

View File

@ -35,6 +35,7 @@ package org.opensearch.repositories.azure;
import com.azure.core.http.HttpMethod;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.http.rest.PagedResponse;
import com.azure.core.http.rest.Response;
import com.azure.core.util.Context;
import com.azure.storage.blob.BlobClient;
@ -51,6 +52,7 @@ import com.azure.storage.blob.models.ListBlobsOptions;
import com.azure.storage.blob.options.BlobParallelUploadOptions;
import com.azure.storage.common.implementation.Constants;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.util.Throwables;
@ -82,6 +84,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicLong;
@ -217,7 +220,23 @@ public class AzureBlobStore implements BlobStore {
final ListBlobsOptions listBlobsOptions = new ListBlobsOptions().setPrefix(path);
SocketAccess.doPrivilegedVoidException(() -> {
for (final BlobItem blobItem : blobContainer.listBlobs(listBlobsOptions, timeout())) {
String continuationToken = null;
do {
// Fetch one page at a time, others are going to be fetched by continuation token
// TODO: reconsider reverting to simplified approach once https://github.com/Azure/azure-sdk-for-java/issues/26064
// gets addressed.
final Optional<PagedResponse<BlobItem>> pageOpt = blobContainer.listBlobs(listBlobsOptions, timeout())
.streamByPage(continuationToken)
.findFirst();
if (!pageOpt.isPresent()) {
// No more pages, should never happen
break;
}
final PagedResponse<BlobItem> page = pageOpt.get();
for (final BlobItem blobItem : page.getValue()) {
// Skipping prefixes as those are not deletable and should not be there
assert (blobItem.isPrefix() == null || !blobItem.isPrefix()) : "Only blobs (not prefixes) are expected";
@ -260,7 +279,12 @@ public class AzureBlobStore implements BlobStore {
}
});
}
// Fetch next continuation token
continuationToken = page.getContinuationToken();
} while (StringUtils.isNotBlank(continuationToken));
});
if (outstanding.decrementAndGet() == 0) {
result.onResponse(null);
}
@ -301,7 +325,23 @@ public class AzureBlobStore implements BlobStore {
.setPrefix(keyPath + (prefix == null ? "" : prefix));
SocketAccess.doPrivilegedVoidException(() -> {
for (final BlobItem blobItem : blobContainer.listBlobsByHierarchy("/", listBlobsOptions, timeout())) {
String continuationToken = null;
do {
// Fetch one page at a time, others are going to be fetched by continuation token
// TODO: reconsider reverting to simplified approach once https://github.com/Azure/azure-sdk-for-java/issues/26064
// gets addressed
final Optional<PagedResponse<BlobItem>> pageOpt = blobContainer.listBlobsByHierarchy("/", listBlobsOptions, timeout())
.streamByPage(continuationToken)
.findFirst();
if (!pageOpt.isPresent()) {
// No more pages, should never happen
break;
}
final PagedResponse<BlobItem> page = pageOpt.get();
for (final BlobItem blobItem : page.getValue()) {
// Skipping over the prefixes, only look for the blobs
if (blobItem.isPrefix() != null && blobItem.isPrefix()) {
continue;
@ -315,6 +355,9 @@ public class AzureBlobStore implements BlobStore {
blobsBuilder.put(name, new PlainBlobMetadata(name, properties.getContentLength()));
}
// Fetch next continuation token
continuationToken = page.getContinuationToken();
} while (StringUtils.isNotBlank(continuationToken));
});
return MapBuilder.newMapBuilder(blobsBuilder).immutableMap();
@ -330,7 +373,23 @@ public class AzureBlobStore implements BlobStore {
.setPrefix(keyPath);
SocketAccess.doPrivilegedVoidException(() -> {
for (final BlobItem blobItem : blobContainer.listBlobsByHierarchy("/", listBlobsOptions, timeout())) {
String continuationToken = null;
do {
// Fetch one page at a time, others are going to be fetched by continuation token
// TODO: reconsider reverting to simplified approach once https://github.com/Azure/azure-sdk-for-java/issues/26064
// gets addressed
final Optional<PagedResponse<BlobItem>> pageOpt = blobContainer.listBlobsByHierarchy("/", listBlobsOptions, timeout())
.streamByPage(continuationToken)
.findFirst();
if (!pageOpt.isPresent()) {
// No more pages, should never happen
break;
}
final PagedResponse<BlobItem> page = pageOpt.get();
for (final BlobItem blobItem : page.getValue()) {
// Skipping over the blobs, only look for prefixes
if (blobItem.isPrefix() != null && blobItem.isPrefix()) {
// Expecting name in the form /container/keyPath.* and we want to strip off the /container/
@ -341,7 +400,9 @@ public class AzureBlobStore implements BlobStore {
blobsBuilder.add(name);
}
}
;
// Fetch next continuation token
continuationToken = page.getContinuationToken();
} while (StringUtils.isNotBlank(continuationToken));
});
return Collections.unmodifiableMap(

View File

@ -208,6 +208,7 @@ public class AzureHttpHandler implements HttpHandler {
}
list.append("</Blobs>");
list.append("<NextMarker />");
list.append("</EnumerationResults>");
byte[] response = list.toString().getBytes(StandardCharsets.UTF_8);