-
Notifications
You must be signed in to change notification settings - Fork 180
Align big5 ppl queries to dsl #4198
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
Conversation
Signed-off-by: Louis Chu <[email protected]>
Signed-off-by: Louis Chu <[email protected]>
| source = big5 | ||
| | where `@timestamp` >= '2022-12-30 00:00:00' and `@timestamp` < '2023-01-01 03:00:00' | ||
| | stats count() by `process.name`, `event.id`, `cloud.region` | ||
| | where `@timestamp` >= '2023-01-05 00:00:00' and `@timestamp` < '2023-01-05 05:00:00' |
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.
{
"size": 0,
"query": {
"range": {
"@timestamp": {
"gte": "2022-12-30T00:00:00",
"lt": "2023-01-01T03:00:00"
}
}
},
"aggs": {
"important_terms": {
"multi_terms": {
"terms": [
{
"field": "process.name"
},
{
"field": "event.id"
},
{
"field": "cloud.region"
}
]
}
}
}
}
This correspond DSL query to PPL. Why change timerange?
| | where `process.name` = 'systemd' | ||
| and `metrics.size` >= 1 | ||
| and `metrics.size` <= 1000 | ||
| | where `process.name` = 'systemd' and `metrics.size` >= 1 and `metrics.size` <= 100 |
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.
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.
Looks like there are difference sources of Big5 DSL. The original PPL was translated from the DSL listed above. But @noCharger seems find another source of Big5 DSL.
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.
@penghuo @LantaoJin The actual benchmark queries are defined https://github.com/opensearch-project/opensearch-benchmark-workloads/blob/main/big5/operations/default.json, and invoked here https://github.com/opensearch-project/opensearch-benchmark-workloads/blob/main/big5/test_procedures/default.json#L8. Hence we need to align the queries.
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.
|
@noCharger can you call out the true source of DSL in the description? Seems there are some different in benchmark repo. So we should call out the source we aligned. |
Yes it's already in description. |
|
This PR is stalled because it has been open for 30 days with no activity. |
|
@noCharger resolve conflict |
Signed-off-by: Louis Chu <[email protected]>
|
|
This PR is stalled because it has been open for 2 weeks with no activity. |
|
closiing due to #4668 |
Description
Align source ppl query to big5 dsl used for benchmark. Verified query results are the same between PPL and DSL on modified queries in this PR, besides issues identified:
multi_terms_keywordquery returns different results from dsl because it converts to composite agg instead of multi_terms agg. Track [FEATURE] PPL should support terms aggregation with multiple buckets #4208auto_date_histogramqueries return different results from dsl because it converts to composite agg instead of auto_date_histogram agg. Track [FEATURE] Span() / bin should support auto_date_histogram aggregation #4210DSL https://github.com/opensearch-project/opensearch-benchmark-workloads/blob/main/big5/operations/default.json
Related Issues
opensearch-project/opensearch-benchmark-workloads#691
Check List
- [ ] New functionality has been documented.- [ ] New functionality has javadoc added.- [ ] New functionality has a user manual doc added.- [ ] New PPL command checklist all confirmed.- [ ] API changes companion pull request created.--signoffor-s.- [ ] Public documentation issue/PR created.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.