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

Provide variation of the workload(s) with unsigned_long data type #86

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

reta
Copy link
Contributor

@reta reta commented Jul 12, 2023

Description

Provide variation of the workload(s) with unsigned_long data type

Issues Resolved

Closes #85

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

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@@ -80,10 +80,10 @@
"scaling_factor": 100,
"type": "scaled_float"
},
"trip_distance": {
"trip_distance": {%- if trip_distance_type is defined %} {{ trip_distance_type | tojson }} {%- else %} {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@IanHoang would appreciate your feedback on this solution: it turned out we don't need change in test procedure - the default one works just fine (it has aggregations over trip_distrance) but what we need it the mapping type change for trip_distance. Using workload params seems like one of the ways to achieve that (without copy/pasting the whole index).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

workload-params.json:

{
    "trip_distance_type": {
        "type": "unsigned_long"
    }
}

And use like that: opensearch-benchmark execute_test --pipeline=benchmark-only --workload=nyc_taxis --target-hosts=localhost:9200 --workload-params=workload-params.json

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you can use workload-params to achieve this, but it would be helpful to update the README with an example, after you decide what to name the trip_distance_type literal.

@@ -73,6 +73,7 @@ This workload allows to overwrite the following parameters using `--workload-par
* `error_level` (default: "non-fatal"): Available for bulk operations only to specify ignore-response-error-level.
* `target_throughput` (default: default values for each operation): Number of requests per second, `none` for no limit.
* `search_clients`: Number of clients that issues search requests.
* `trip_distance_type` (default: { "scaling_factor": 100, "type": "scaled_float" }): The `trip_distance` field type mapping
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not seem right. The literal trip_distance_type refers not just to a type, but also includes the scaling factor, so it should probably be renamed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

trip_distance_type

Sure, like trip_distance_field_mapping or trip_distance_mapping?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The former seems more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So trip_distance_mapping it is, any more comments? thank you!

@@ -80,10 +80,10 @@
"scaling_factor": 100,
"type": "scaled_float"
},
"trip_distance": {
"trip_distance": {%- if trip_distance_type is defined %} {{ trip_distance_type | tojson }} {%- else %} {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you can use workload-params to achieve this, but it would be helpful to update the README with an example, after you decide what to name the trip_distance_type literal.

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
}
```

Save it as `params.json` and provide it to Benchmark with `--workload-params="/path/to/params.json"`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change to "OpenSearch Benchmark". There are some references to "Benchmark" for historical reasons, but they will be updated in due course.

Comment on lines 78 to 87
### Example of specifying parameters using `--workload-params`:

Example:
```json
{
"trip_distance_mapping": {
"type": "unsigned_long"
}
}
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rename the section title to something like "Specifying Workload Parameters" and add a link to it from the previous section:

This workload allows [specifying the following parameters](#specifying-workload-parameters) using the `-workload-parameters` option to OSB.

You should also indicate that if it is a simple entry, something like --workload-params=search_clients:2 will suffice.

@reta reta force-pushed the issue-85 branch 2 times, most recently from 1816fff to e0cb121 Compare July 27, 2023 12:36
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@reta
Copy link
Contributor Author

reta commented Aug 2, 2023

@gkamat anything else is holding us off? thank you

Copy link
Collaborator

@gkamat gkamat left a comment

Choose a reason for hiding this comment

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

Thank you for incorporating the suggestions.

@gashutos
Copy link
Contributor

gashutos commented Aug 9, 2023

@reta @gkamat can we merge this ?

@reta
Copy link
Contributor Author

reta commented Aug 9, 2023

@reta @gkamat can we merge this ?

I cannot :)

@IanHoang IanHoang merged commit 4499d69 into opensearch-project:main Aug 9, 2023
2 checks passed
gkamat pushed a commit to gkamat/opensearch-benchmark-workloads that referenced this pull request Aug 10, 2023
…ensearch-project#86)

* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
gkamat pushed a commit to gkamat/opensearch-benchmark-workloads that referenced this pull request Aug 10, 2023
…ensearch-project#86)

* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
gkamat pushed a commit that referenced this pull request Aug 11, 2023
* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
gkamat pushed a commit to gkamat/opensearch-benchmark-workloads that referenced this pull request Aug 13, 2023
…ensearch-project#86)

* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
gkamat pushed a commit that referenced this pull request Aug 14, 2023
* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
gkamat pushed a commit to gkamat/opensearch-benchmark-workloads that referenced this pull request Nov 18, 2023
…ensearch-project#86)

* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
gkamat pushed a commit to gkamat/opensearch-benchmark-workloads that referenced this pull request Nov 18, 2023
…ensearch-project#86)

* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
gkamat pushed a commit to gkamat/opensearch-benchmark-workloads that referenced this pull request Nov 18, 2023
…ensearch-project#86)

* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
gkamat pushed a commit that referenced this pull request Nov 20, 2023
* Provide variation of the workload(s) with unsigned_long data type

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Govind Kamat <govkamat@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Provide variation of the workload(s) with unsigned_long data type
4 participants