Skip to content

Performance Framework Investigation - #32020

Merged
alzimmermsft merged 19 commits into
Azure:mainfrom
alzimmermsft:AzEng_PerfTestInvestigation
Nov 11, 2022
Merged

Performance Framework Investigation#32020
alzimmermsft merged 19 commits into
Azure:mainfrom
alzimmermsft:AzEng_PerfTestInvestigation

Conversation

@alzimmermsft

@alzimmermsft alzimmermsft commented Nov 8, 2022

Copy link
Copy Markdown
Member
  • Replaced usage of Flux.interval with Timer to manage printing performance stats while running tests. Flux.interval ran on the same thread pool as the performance tests, using Timer creates a new thread to manage that. This should be a large impact but just keeps the test resources a bit cleaner.
  • Removed usage of extra parameters in ParallelFlux.flatMap as they didn't do anything as the number of operations in-flight at any time would always be parallel.
  • Switched everything parallelized to Schedulers.parallel as that is Reactor's recommendation for handling parallelization.
  • Removed unused code from Storage's perf tests.
  • Changed how download blob resource preparation is done to send the blob as a single request. This fixes a similar issue seem in Change how List Blobs Perf Creates Blobs #31817 where this resource creation could pin all network requests to the same IO thread handling them, resulting in much lower performance than expected in testing (as one thread cannot keep up with the request load).
  • Changed blob creation in list blob to use API that only makes one network request instead of two.

@ghost ghost added the Storage Storage Service (Queues, Blobs, Files) label Nov 8, 2022
@azure-sdk

Copy link
Copy Markdown
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-storage-blob

@alzimmermsft
alzimmermsft force-pushed the AzEng_PerfTestInvestigation branch from 99b0b17 to ab7a11b Compare November 10, 2022 16:21
@alzimmermsft
alzimmermsft marked this pull request as ready for review November 11, 2022 13:08
.flatMap(iteration -> blobContainerAsyncClient.getBlobAsyncClient("getblobstest-" + UUID.randomUUID())
.upload(Flux.empty(), null), false, Math.min(options.getParallel(), 1000 / options.getParallel()), 1)
.getBlockBlobAsyncClient()
.upload(Flux.empty(), 0L), false, Math.min(1000 / parallel, parallel), 1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this instance of Math.min(1000 / parallel, parallel) still needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the Math.min isn't necessary, this can use false, parallel, 1 instead.

.upload(Flux.empty(), null), false, Math.min(options.getParallel(), 1000 / options.getParallel()), 1)
.getBlockBlobAsyncClient()
.upload(Flux.empty(), 0L), false, Math.min(1000 / parallel, parallel), 1)
.sequential()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of sequential()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All examples of ParallelFlux used in Reactor's samples used sequential so I thought it'd be good to follow the patterns they use.

public Mono<Void> globalSetupAsync() {
return super.globalSetupAsync()
.then(blobAsyncClient.upload(createRandomByteBufferFlux(options.getSize()), null))
.then(blobAsyncClient.upload(createRandomByteBufferFlux(options.getSize()), new ParallelTransferOptions()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment explaining why we are overriding the default upload size and block size? Is 1GB the max, or could we set it even higher (in case we want to start testing blobs > 1GB)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment explaining this

@alzimmermsft
alzimmermsft merged commit eba2881 into Azure:main Nov 11, 2022
@alzimmermsft
alzimmermsft deleted the AzEng_PerfTestInvestigation branch November 11, 2022 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants