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

Adds documentation for new OSB flags #6403

Merged
merged 6 commits into from
Feb 22, 2024

Conversation

peteralfonsi
Copy link
Contributor

Description

Adds documentation for new arguments added to OpenSearch Benchmark in these PRs:
opensearch-project/opensearch-benchmark#441
opensearch-project/opensearch-benchmark#449
opensearch-project/opensearch-benchmark#455

Issues Resolved

N/A

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@peteralfonsi
Copy link
Contributor Author

@Naarcha-AWS - @IanHoang mentioned you would be a good person to tag for reviewing this PR

@hdhalter hdhalter added the 3 - Tech review PR: Tech review in progress label Feb 14, 2024
<!-- vale on -->

Enables randomization of values in range queries, where the values are drawn from standard value functions registered with `register_standard_value_source` in the workload's workload.py. Default is `False`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: For folks who are not familiar, it would be good to add a small blurb defining "standard value functions".

Copy link
Collaborator

Choose a reason for hiding this comment

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

@IanHoang: Do you have a good working definition for "standard value functions"?

Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if there is no such function registered? Please indicate that an error is raised if that is the case.

Copy link
Contributor

Choose a reason for hiding this comment

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

standard value functions can be defined as:

It's a function you call to generate a random pair of values for a certain field.

Source: opensearch-project/opensearch-benchmark#455 (comment)

@peteralfonsi feel free to add more insight if needed. Govind has a good point, we should also address what happens if no function is registered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will add a definition and the case when nothing is registered.

@Naarcha-AWS Naarcha-AWS added 4 - Doc review PR: Doc review in progress backport 2.11 PR: Backport label for 2.11 and removed 3 - Tech review PR: Tech review in progress labels Feb 15, 2024
<!-- vale on -->

Specifies a list of latency percentiles to report after the workload runs. Default is `50,90,99,99.9,99.99,100`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be specified as a number or does median, min or max work as well? This should be documented, since the throughput documentation mentions these terms.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe latency percentiles and throughput percentiles only takes in numbers and not median, min, and max. To do median, min, and max, we'd have to specify 0,50,100. Is this correct @peteralfonsi ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, it only takes numeric percentiles. I will add a sentence about that.

<!-- vale on -->

Specifies a list of throughput percentiles to report after the workload runs, in addition to mean/median/max/min. Default is `None`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this to be specified as floating point numbers? Please elaborate here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It can be taken as an int, float, or str. Based on the code here that Peter added, it converts the user inputs to floats.

<!-- vale on -->

Enables randomization of values in range queries, where the values are drawn from standard value functions registered with `register_standard_value_source` in the workload's workload.py. Default is `False`.
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if there is no such function registered? Please indicate that an error is raised if that is the case.

@peteralfonsi
Copy link
Contributor Author

Apologies for getting back to this PR so late. I've addressed your comments. Let me know if I should change anything else.

@Naarcha-AWS Naarcha-AWS removed the backport 2.11 PR: Backport label for 2.11 label Feb 22, 2024
@Naarcha-AWS Naarcha-AWS added the backport 2.12 PR: Backport label for 2.12 label Feb 22, 2024
@Naarcha-AWS Naarcha-AWS merged commit e1fed49 into opensearch-project:main Feb 22, 2024
4 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 22, 2024
* Adds documentation for new OSB flags

Signed-off-by: Peter Alfonsi <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

* Addressed comments

Signed-off-by: Peter Alfonsi <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

---------

Signed-off-by: Peter Alfonsi <[email protected]>
Signed-off-by: Naarcha-AWS <[email protected]>
Co-authored-by: Peter Alfonsi <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
(cherry picked from commit e1fed49)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Naarcha-AWS added a commit that referenced this pull request Feb 22, 2024
* Adds documentation for new OSB flags



* Apply suggestions from code review



* Addressed comments



* Apply suggestions from code review



---------





(cherry picked from commit e1fed49)

Signed-off-by: Peter Alfonsi <[email protected]>
Signed-off-by: Naarcha-AWS <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Peter Alfonsi <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
oeyh pushed a commit to oeyh/documentation-website that referenced this pull request Mar 14, 2024
* Adds documentation for new OSB flags

Signed-off-by: Peter Alfonsi <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

* Addressed comments

Signed-off-by: Peter Alfonsi <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

---------

Signed-off-by: Peter Alfonsi <[email protected]>
Signed-off-by: Naarcha-AWS <[email protected]>
Co-authored-by: Peter Alfonsi <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Doc review PR: Doc review in progress backport 2.12 PR: Backport label for 2.12
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants