Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Service Bus] Perf tests with the framework #12993

Merged
22 commits merged into from
Jan 29, 2021

Conversation

HarshaNalluru
Copy link
Member

@HarshaNalluru HarshaNalluru commented Dec 21, 2020

New perf folder

  • The new "perf" folder uses the perf framework.
  • With the perf framework, only the send test can be written.
  • This PR adds the send test for track 1 and track 2.
  • This would be useful in comparing track 1 and track 2, as well as for the cross-language comparisons.
  • The track-2 tests depend on the current code on master and would require updates if the source code changes.

To execute the track 2 tests

  1. Build the service-bus package rush build -t service-bus.
  2. Navigate to service-bus folder cd sdk\servicebus\service-bus\.
  3. Create a service-bus namespace and populate the .env file at servicebus\service-bus folder with SERVICEBUS_CONNECTION_STRING variables.
  4. Run the tests as follows
    • batch send
      • npm run perf-test:node -- BatchSendTest --warmup 2 --duration 7 --parallel 2

To execture the track 1 tests

  1. Navigate to test-utils\perfstress folder cd sdk\test-utils\perfstress\
  2. Build the package rush update && rush build -t test-utils-perfstress
  3. Pack the perf package rushx pack
  4. Navigate to service-bus\perf\track-1 folder cd sdk\servicebus\service-bus\perf\track-1.
  5. Install the perf package npm i ..\..\..\..\..\test-utils\perfstress\azure-test-utils-perfstress-1.0.0.tgz
  6. Run npm install to get service-bus V1.
  7. Create a service-bus namespace and a queue with default options. Populate the .env file at servicebus\service-bus folder with SERVICEBUS_CONNECTION_STRING and QUEUE_NAME variables.
  8. Run the tests as follows
    • batch send
      • npm run perf-test:node -- BatchSendTest --warmup 2 --duration 7 --parallel 2

Existing perf tests

  • There is an existing "perf" folder with the tests for send and receive for azure-sb, rhea-promise, service-bus-v1, and service-bus-v7 libraries.
  • These tests show the memory consumption too in the report. They do not use the perf framework. (And the perf framework isn't yet capable of handling the receive scenarios.)
  • Retaining the existing tests for the above reasons.
  • This PR renames the "perf" folder(with existing tests) to "perf-js-libs".

@ghost ghost added the Service Bus label Dec 21, 2020
@@ -10,7 +10,8 @@
"node_modules",
"./types/**/*.d.ts",
"./samples/**/*.ts",
"test/perf/*",
"test/perf-js-libs/*",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I get what goes in perf-js-libs vs perf/track-? Is it just archiving these files?

Copy link
Member Author

Choose a reason for hiding this comment

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

More or less, yes.

perf-js-libs

  • perf-js-libs/azure-sb-package
  • perf-js-libs/rhea-promise
  • perf-js-libs/service-bus-v1
  • perf-js-libs/service-bus-v7

perf/track-*

  • perf/track-1 - v1
  • perf/track-2 - v7

Description

  • perf-js-libs do not use the framework and have the tests for send, streaming receive and batch receive.
  • perf/track-* uses the framework for send test, and currently has the send test only.
    • Since this uses the framework, it helps with comparisons across languages.
  • Framework isn't capable of handling the receive tests, so they have to be standalone tests.

More thoughts

  • Eventually, I intend to have the standalone tests in the perf/track-* folder itself and thus removing the need for perf-js-libs/service-bus-v* folders.
  • The perf-js-libs/azure-sb-package can be removed as it is pretty much outdated at the moment.
  • That leaves us with perf-js-libs/rhea-promise, which could be moved to perf/rhea-promise after updating the send test with the framework and migrating the standalone receive tests.
  • Finally, we'll end up with perf/track-1, perf/track-2 and perf/rhea-promise.

Copy link
Member Author

Choose a reason for hiding this comment

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

await this.sender.sendMessages(
new Array(this.parsedOptions.numberOfMessages.value!).fill(this.sbMessage)
);
await this.sender.sendMessages(this.batch);
Copy link
Member

Choose a reason for hiding this comment

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

Future concern: should we be using .tryAdd() ourselves or are we guaranteed that we'll never exceed the size of a single batch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, makes sense to use tryAdd. I'll put a follow-up PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@richardpark-msft richardpark-msft left a comment

Choose a reason for hiding this comment

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

Still looks good (since my last review!)

@ghost
Copy link

ghost commented Jan 29, 2021

Hello @HarshaNalluru!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit e15c042 into Azure:master Jan 29, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants