-
Notifications
You must be signed in to change notification settings - Fork 313
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
operation-type:composite fails when track_total_hits is false #1881
Comments
We would need a flag to check wether {
"name": "async-search",
"operation-type": "submit-async-search",
"index": "hotfrozenlogs",
"request-params": {
"track_total_hits": false //here
},
"body": {
"track_total_hits": false //or here
... |
Here is an agnostic (to track_total_hits) approach that works just by checking if class GetAsyncSearch(Runner):
async def __call__(self, es, params):
success = True
searches = mandatory(params, "retrieve-results-for", self)
request_params = params.get("request-params", {})
stats = {}
for search_id, search in async_search_ids(searches):
response = await es.async_search.get(id=search_id, params=request_params)
is_running = response["is_running"]
success = success and not is_running
if not is_running:
stats[search] = {
"timed_out": response["response"]["timed_out"],
"took": response["response"]["took"],
}
if "total" in response["response"]["hits"].keys():
stats[search]["hits"] = response["response"]["hits"]["total"]["value"]
stats[search]["hits_relation"] = response["response"]["hits"]["total"]["relation"] Another option could be to check in the class SubmitAsyncSearch(Runner):
async def __call__(self, es, params):
...
#track_total_hits is true by default
CompositeContext.put("track_total_hits", True)
if "track_total_hits" in params.get("body").keys():
CompositeContext.put("track_total_hits", bool(eval(params.get("body").get("track_total_hits").capitalize())))
if "track_total_hits" in request_params.keys():
CompositeContext.put("track_total_hits", bool(eval(request_params.get("track_total_hits").capitalize()))) class GetAsyncSearch(Runner):
...
if CompositeContext.get("track_total_hits"):
stats[search]["hits"] = response["response"]["hits"]["total"]["value"]
stats[search]["hits_relation"] = response["response"]["hits"]["total"]["relation"] @gareth-ellis what do you think? |
I like the agnostic change - it protects us from crashing out if some other parameter were to change the output behaviour too - presuming everything keeps working as expected, that is |
It looks like the tests are also checking for total, I will fix those too. |
We are using a composite operation to benchmark searchable snapshots using async search like below:
And its failing with:
esrally.exceptions.RallyError: Cannot run task [date_histogram_by_day-90d-hotfrozen]: Cannot execute [user-defined context-manager enabled runner for [composite]]. Provided parameters are: ['operation-type', 'name', 'iterations', 'requests', 'include-in-reporting']. Error: ['total'].
The log shows GetAsyncSearch is trying to read the
total
in:rally/esrally/driver/runner.py
Line 2530 in 543b4dc
The text was updated successfully, but these errors were encountered: