Add perf testing for singleton client usage #4
Conversation
| BlobServiceClient storageClient = blobServiceClientBuilder.buildClient(); | ||
|
|
||
| BlobContainerClient blobContainerClient = storageClient.createBlobContainer("perfupload" + UUID.randomUUID()); | ||
| BlobProperties blobProperties = blobContainerClient.getBlobClient(blobName).getProperties(); |
There was a problem hiding this comment.
@alzimmermsft This gives me a 404 for BlobNotFound for this test case but not the UploadFile . Is getProperties api selectively available?
There was a problem hiding this comment.
So, upload from file will create a blob but I'm guessing it didn't exist previously, this performance test will need to create the blob in setup first
| assert blobProperties.getCreationTime() != null; | ||
| assert blobProperties.getExpiresOn() != null; |
There was a problem hiding this comment.
nit: no need to do validation logic once we're happy with the validity of the perf test
| String blobName = "perfblobtest-" + UUID.randomUUID(); | ||
|
|
||
| BlobServiceClient storageClient = blobServiceClientBuilder.buildClient(); | ||
|
|
||
| BlobContainerClient blobContainerClient = storageClient.createBlobContainer("perfupload" + UUID.randomUUID()); | ||
| BlobClient blobClient = blobContainerClient.getBlobClient(blobName); | ||
| blobClient.uploadFromFile(filePath, true); |
There was a problem hiding this comment.
For both tests, and the sync and async variants, we'll want to either add a configuration flag to share the singleton HttpClient or to use a new one on each call. Or does it always share the HttpClient given which branch this PR is targeting?
There was a problem hiding this comment.
yes, Used different dependency versions, a locally compiled one vs already deployed.
we'll want to either add a configuration flag to share the singleton HttpClient
Idk, if we support using a configuration flag to specify to use or not use a shared http client.
samvaity
force-pushed
the
share-client-perf
branch
from
ee398f8
to
ea06c88
Compare
| <dependency> | ||
| <groupId>com.azure</groupId> | ||
| <artifactId>azure-storage-blob</artifactId> | ||
| <version>12.13.0-beta.1</version> <!-- {x-version-update;com.azure:azure-storage-blob;dependency} --> | ||
| </dependency> |
There was a problem hiding this comment.
I'd also add the current version of azure-core as a dependency to this. Reason being that it'll override the dependency in any downstream libraries being used in testing to provide the following:
- Changes to Core will be performance tested.
- Regressions in Core will be more likely to be found.
- It will test the latest Core which performance tests should be doing.
| @@ -83,7 +83,7 @@ | |||
| <dependency> | |||
| <groupId>com.azure</groupId> | |||
| <artifactId>azure-core-http-netty</artifactId> | |||
| <version>1.11.4</version> <!-- {x-version-update;com.azure:azure-core-http-netty;dependency} --> | |||
| <version>1.12.0-beta.1</version> <!-- {x-version-update;unreleased_com.azure:azure-core-http-netty;dependency} --> | |||
There was a problem hiding this comment.
I would revert this in favor of the performance test overriding the version of Netty being used.
| if (useSharedClient) { | ||
| BlobServiceClient blobServiceClient = blobServiceClientBuilder.buildClient(); | ||
| performGetProperties(blobServiceClient); | ||
| } else { | ||
| // use a customized http client | ||
| HttpClientOptions clientOptions = new HttpClientOptions().setMaximumConnectionPoolSize(50); | ||
| HttpClient client1 = new NettyAsyncHttpClientProvider().createInstance(clientOptions); | ||
|
|
||
| BlobServiceClient blobServiceClient = blobServiceClientBuilder | ||
| .httpClient(client1) | ||
| .buildClient(); | ||
|
|
||
| performGetProperties(blobServiceClient); | ||
| } | ||
| } |
There was a problem hiding this comment.
So, given the way Storage works both of these will end up sharing the HttpClient. Storage passes the HttpPipeline of a parent resource (service client -> container client, container client -> blob client) to the child resource. It may be better to pass a Supplier<BlobContainerClient>, or Function<String, BlobContainerClient>, where the one for a shared client is passing the pre-built client and the one for a non-shared client goes through a new builder.
UploadFile Test (Singleton client)
UploadFile Test