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

Honor ingest-percentage for bulks #768

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit we honor the parameter ingest-percentage again for bulk
requests. Before #763, the method size() was used to determine upfront when a
parameter source is exhausted. When we changed the API then in #763 the
parameter source's progress indication was instead done with the
percent_completed property but this is only informational and the parameter
source is required to raise StopIteration to indicate completion. This means
that ingest-percentage was not considered anymore for bulks but instead we
always read the input file(s) completely and then terminated with
StopIteration.

Relates #763

With this commit we honor the parameter `ingest-percentage` again for bulk
requests. Before elastic#763, the method `size()` was used to determine upfront when a
parameter source is exhausted. When we changed the API then in elastic#763 the
parameter source's progress indication was instead done with the
`percent_completed` property but this is only informational and the parameter
source is required to raise `StopIteration` to indicate completion. This means
that `ingest-percentage` was not considered anymore for bulks but instead we
always read the input file(s) completely and then terminated with
`StopIteration`.

Relates elastic#763
@danielmitterdorfer danielmitterdorfer added bug Something's wrong :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. labels Sep 10, 2019
@danielmitterdorfer danielmitterdorfer added this to the 1.4.0 milestone Sep 10, 2019
Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the quick fix.

@dliappis
Copy link
Contributor

For the record, this was observed with a reproduction like:

./rally --distribution-version=7.3.0 --track=geonames --challenge=append-no-conflicts-index-only --track-params='ingest_percentage:50'

where progress for index-append would exceed 100%.

@danielmitterdorfer
Copy link
Member Author

Thanks for the review!

@danielmitterdorfer danielmitterdorfer merged commit fa26327 into elastic:master Sep 10, 2019
@danielmitterdorfer danielmitterdorfer deleted the bulk-ingest-percentage branch September 10, 2019 15:19
novosibman pushed a commit to novosibman/rally that referenced this pull request Oct 2, 2019
With this commit we honor the parameter `ingest-percentage` again for bulk
requests. Before elastic#763, the method `size()` was used to determine upfront when a
parameter source is exhausted. When we changed the API then in elastic#763 the
parameter source's progress indication was instead done with the
`percent_completed` property but this is only informational and the parameter
source is required to raise `StopIteration` to indicate completion. This means
that `ingest-percentage` was not considered anymore for bulks but instead we
always read the input file(s) completely and then terminated with
`StopIteration`.

Relates elastic#763
Relates elastic#768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's wrong :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants