-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Adds performance tests for Event Hubs. #21822
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left some comments but not required for the 1st usable version.
@Override | ||
public TransportType convert(String s) { | ||
if (s == null) { | ||
throw new ParameterException(getErrorString("null", "AmqpTransportType")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about default value AMQP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is to use AmqpTransportType.AMQP. It only tries to parse this if you pass in the parameter --transportType
. And if you didn't pass a value, I think that should be a user error because it's not what you expected.
} | ||
|
||
private String getResults(int index, EventsCounter eventsCounter) { | ||
final double seconds = eventsCounter.elapsedTime() * 0.000000001; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/1_000_000_000.0
might be easier to count the number of zeros
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yeah. I always thought multiplication was faster for processors than division. But maybe this is moot now.
...enthubs-track2-perf/src/main/java/com/azure/messaging/eventhubs/perf/EventProcessorTest.java
Show resolved
Hide resolved
...eventhubs-track2-perf/src/main/java/com/azure/messaging/eventhubs/perf/EventHubsOptions.java
Show resolved
Hide resolved
* @see ReceiveEventsTest | ||
* @see EventProcessorTest | ||
*/ | ||
public class EventHubsReceiveOptions extends EventHubsOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may add prefetch in the options so we can test it. It's one of the most important factors for receiving (including event processor) performance.
But it requires us to change the SDK code to use the prefetch for request size instead of just the first request.
* @see EventProcessorTest | ||
*/ | ||
public class EventProcessorOptions extends EventHubsReceiveOptions { | ||
@Parameter(names = {"-scs", "--storageConnectionString"}, description = "Connection string for Storage account.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we allow a test that use sample checkpoint store without a storage container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that if we want something comparable, we need to do the same implementation for Track 1... I had tried it for a few hours but it was too confusing to continue.
private String storageEndpoint; | ||
|
||
@Parameter(names = {"-e", "--eventsToSend"}, description = "Number of events to send per partition.") | ||
private int eventsToSend = 100000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May add event payload size as a parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Base class PerfStressOptions expose a .getSize() (-s, --size) parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use this in ServiceTest line 62.
…into performance-tests
Machinelearningservices microsoft.machine learning services 2022 12 01 preview (Azure#21761) * Adds base for updating Microsoft.MachineLearningServices from version preview/2022-10-01-preview to version 2022-12-01-preview * Updates readme * Updates API version in new specs and examples * Add Dec API Registries Swagger (Azure#21419) * add december registries swagger + examples * add status code 202 in examples * fix 202 examples * fixes * fixes * fix * add 202 back in for put/patch Co-authored-by: Komal Yadav <[email protected]> * remove location (Azure#21430) Co-authored-by: Komal Yadav <[email protected]> * remove readonly flag on schedules property for CI (Azure#21653) Co-authored-by: Naman Agarwal <[email protected]> * add missing workspace properties (Azure#21725) * December preview updating mfe.json specs (Azure#21510) * December preview updating mfe.json specs * MFE Dec 2022 Preview API - Adding logbase * MFE 2022-12-01-preview swagger spec model validation fix * MFE 2022-12-01-preview swagger spec model validation fix, add missing location * MFE 2022-12-01-preview swagger spec model validation - typo fix * MFE 2022-12-01-preview swagger spec model validation - fix api version in automljob example * MFE 2022-12-01-preview swagger spec model validation - fix for multiselectenabled error * MFE 2022-12-01-preview swagger spec model validation - fix for multiselectenabled error * Fix for 1006 - RemovedDefinition (RecurrenceTrigger,CronTrigger) (Azure#21822) * fix ReadonlyPropertyChanged of MLC (Azure#21814) Co-authored-by: Bingchen Li <[email protected]> * fixed custom-words conflict (Azure#21829) * fix custom-words conflict merge (Azure#21830) * example fix (INVALID_REQUEST_PARAMETER) (Azure#21832) Co-authored-by: Ivaliy Ivanov <[email protected]> * example fix, use correct api preview version - (INVALID_REQUEST_PARAMETER) (Azure#21833) Co-authored-by: Ivaliy Ivanov <[email protected]> * Revert breaking change for MLC swagger 2022-12-01-preview (Azure#21885) Co-authored-by: Bingchen Li <[email protected]> * Revert Connection Category back to enum. (Azure#21939) * revert provisioning state change (Azure#21940) * remove body (Azure#21978) Co-authored-by: Komal Yadav <[email protected]> * Addressed comments, added x-ms-long-running-operation to a patch call (Azure#22005) * Addressed comments, added x-ms-long-running-operation to a patch call * fix examples for patch - remove body * fixed formatting * Ivalbert fix patch2 (Azure#22006) * Addressed comments, added x-ms-long-running-operation to a patch call * fix examples for patch - remove body * fixed formatting * fixed formatting * Updated custom words (Azure#22262) * Fixed prettier errors (Azure#22237) * fixed examples for LRO_RESPONSE_HEADER check (Azure#22293) * fixed examples for LRO_RESPONSE_HEADER check (Azure#22294) * Example fix - OBJECT_MISSING_REQUIRED_PROPERTY - Missing required property: triggerType (Azure#22317) --------- Co-authored-by: Komal Yadav <[email protected]> Co-authored-by: Komal Yadav <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: ZhidaLiu <[email protected]> Co-authored-by: libc16 <[email protected]> Co-authored-by: Bingchen Li <[email protected]> Co-authored-by: Ivaliy Ivanov <[email protected]>
Related #20841
Fixes #20791