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

Improve response checks in bulk API runner #207

Closed
danielmitterdorfer opened this issue Jan 27, 2017 · 10 comments
Closed

Improve response checks in bulk API runner #207

danielmitterdorfer opened this issue Jan 27, 2017 · 10 comments
Labels
enhancement Improves the status quo :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. :Metrics How metrics are stored, calculated or aggregated
Milestone

Comments

@danielmitterdorfer
Copy link
Member

At the moment, Rally checks for bulk errors by inspecting the errors property of the bulk response (and adds it to the request meta-data).

On errors, we should improve the inspection and also report more meta-data (like items with failed shards etc.)

@danielmitterdorfer danielmitterdorfer added :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. :Metrics How metrics are stored, calculated or aggregated enhancement Improves the status quo labels Jan 27, 2017
@danielmitterdorfer danielmitterdorfer added this to the 0.5.0 milestone Jan 27, 2017
@bleskes
Copy link

bleskes commented Jan 27, 2017

note that failing replicas will not bubble up as an error. You need to look at the failures section of the _shards header of each item.

@danielmitterdorfer
Copy link
Member Author

Thanks for the pointer. In that case I think we should add this as an option which needs to be enabled by the user in the track.

@bleskes
Copy link

bleskes commented Jan 27, 2017

maybe we should improve the bulk response to bubble this up. I think it has merit for exactly the reason you think of making it optional - people won't have to parse the response.

@danielmitterdorfer
Copy link
Member Author

maybe we should improve the bulk response to bubble this up.

This would definitely help in this case.

The reason I wanted to make response parsing optional is to avoid adding any bottlenecks in the load test driver. I did not check yet how long it would take to extract these data (for different bulk sizes) and my gut feeling tells me that the overhead is negligible but I'd rather measure it first and base my decision on hard numbers. :)

@danielmitterdorfer danielmitterdorfer self-assigned this Feb 7, 2017
@danielmitterdorfer
Copy link
Member Author

Rally will return a structure like:

{
  "weight": 5000,
  "unit": "docs",
  "bulk-size": 5000,
  "success": true,
  "success-count": 5000,
  "error-count": 0,
  "ops": {
    "index": {
      "item-count": 5000,
      "created": 5000
    }
  },
  "shards_histogram": [
    {
      "item-count": 5000,
      "shards": {
        "total": 2,
        "successful": 2,
        "failed": 0
      }
    }
  ]
}

when the new parameter detailed-results is set to true in the track specification. If the value is false, it will return instead the following meta-data:

{
  "weight": 5000,
  "unit": "docs",
  "bulk-size": 5000,
  "success": true,
  "success-count": 5000,
  "error-count": 0
}

The default value will be false as this feature adds very significant overhead due to the need to iterate over the complete response structure. The benchmarks (which can be run with make benchmark) indicate a slowdown by a factor greater than 1.000: Whereas we can return within single digit microseconds when detailed-results is false, the call needs several milliseconds when detailed-results is true. Whether this is acceptable needs to be decided on a case-by-case basis (by looking at typical response times of the bulk operation in a benchmark) and to avoid that users run unintentionally into trouble I decided to set it to false by default.

@danielmitterdorfer danielmitterdorfer removed their assignment Feb 7, 2017
@danielmitterdorfer
Copy link
Member Author

@bleskes
Copy link

bleskes commented Feb 8, 2017

@danielmitterdorfer does it mean you gave up on:

maybe we should improve the bulk response to bubble this up. I think it has merit for exactly the reason you think of making it optional - people won't have to parse the response.

@danielmitterdorfer
Copy link
Member Author

@bleskes I did not gave up but Rally works with ES 1.x, 2.x, 5.x and 6.x and I needed to implement a solution that works for all of them. We can still implement this in ES though. I can create a ticket later.

@bleskes
Copy link

bleskes commented Feb 9, 2017

@danielmitterdorfer makes total sense. Thanks for explaining.

@danielmitterdorfer
Copy link
Member Author

danielmitterdorfer commented Feb 13, 2017

I've created elastic/elasticsearch#23143 now as a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. :Metrics How metrics are stored, calculated or aggregated
Projects
None yet
Development

No branches or pull requests

2 participants