Skip to content

Change how List Blobs Perf Creates Blobs - #31817

Merged
alzimmermsft merged 10 commits into
Azure:mainfrom
alzimmermsft:AzPerf_ChangeListingCreation
Nov 1, 2022
Merged

Change how List Blobs Perf Creates Blobs#31817
alzimmermsft merged 10 commits into
Azure:mainfrom
alzimmermsft:AzPerf_ChangeListingCreation

Conversation

@alzimmermsft

@alzimmermsft alzimmermsft commented Oct 28, 2022

Copy link
Copy Markdown
Member

Description

Another attempt at removing the need for Schedulers.boundedElastic() in AysncRestProxy. The change stems from using logging in Reactor Netty to find that the same EPoll thread, in Linux, creates all connections to the Storage account. This may be from blob creation happening sequentially, making it parallel forces multiple EPoll threads to create the connections as multiple requests with happen at the same time making it impossible for one thread to create all connections.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@alzimmermsft alzimmermsft self-assigned this Oct 28, 2022
@ghost ghost added Azure.Core azure-core Storage Storage Service (Queues, Blobs, Files) labels Oct 28, 2022
@azure-sdk

Copy link
Copy Markdown
Collaborator

API change check

API changes are not detected in this pull request.

private static double getOperationsPerSecond(PerfTestBase<?>[] tests) {
return IntStream.range(0, tests.length)
.mapToDouble(i -> tests[i].getCompletedOperations() / (((double) tests[i].lastCompletionNanoTime) / NANOSECONDS_PER_SECOND))
.sum();

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.

This had a fun issue where Double.NaN being added resulted in the value being Double.NaN.

@alzimmermsft
alzimmermsft enabled auto-merge (squash) November 1, 2022 16:19
@alzimmermsft
alzimmermsft merged commit 906f0ad into Azure:main Nov 1, 2022
@alzimmermsft
alzimmermsft deleted the AzPerf_ChangeListingCreation branch November 1, 2022 16:26
mikeharder added a commit to mikeharder/azure-sdk-for-java that referenced this pull request Nov 8, 2022
…changes to Perf Common (Azure#31817)"

This partially reverts commit 906f0ad.
mikeharder added a commit to mikeharder/azure-sdk-for-java that referenced this pull request Nov 8, 2022
mikeharder added a commit to mikeharder/azure-sdk-for-java that referenced this pull request Nov 8, 2022
sink.complete();
}
})
.flatMap(ignored -> {

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.

Why do we need to check if the timer expired twice? The timer is approximate so running one extra operation should be fine. Returning a result of "0" won't increase completedOperations, but it will update lastCompletionNanoTime, which seems wrong.

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.

I'll revert this in #32020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core azure-core Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants