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

Run a task completely even without time-periods #763

Conversation

danielmitterdorfer
Copy link
Member

There are multiple conditions how a task can be completed. Either the user
explicitly specifies a maximum number of iterations or a time-period or a task
is implicitly completed when the parameter source is exhausted (e.g. when
reading parameters from a file). In the latter case it is not necessary to
provide any indication when a task is complete because the parameter source can
determine it. However, so far this was required.

With this commit we introduce a change in the parameter source API so that it is
possible to determine this case and also provide proper progress indication.
Furthermore we adapt all parameter sources that come with Rally out of the box
and provide guidance for users how to change their own parameter sources.

There are multiple conditions how a task can be completed. Either the user
explicitly specifies a maximum number of iterations or a time-period or a task
is implicitly completed when the parameter source is exhausted (e.g. when
reading parameters from a file). In the latter case it is not necessary to
provide any indication when a task is complete because the parameter source can
determine it. However, so far this was required.

With this commit we introduce a change in the parameter source API so that it is
possible to determine this case and also provide proper progress indication.
Furthermore we adapt all parameter sources that come with Rally out of the box
and provide guidance for users how to change their own parameter sources.
@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 5, 2019
@danielmitterdorfer danielmitterdorfer added this to the 1.4.0 milestone Sep 5, 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! I left a comment about clarifying in the docs how to stop a parameter source with an infinite schedule.

return
if task_progress_control.infinite:
logger.info("Parameter source will determine when the schedule for [%s] terminates.", task_name)
param_source_knows_progress = hasattr(params, "percent_completed")
Copy link
Contributor

Choose a reason for hiding this comment

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

I love the name of this variable 👍

* ``params(self)``: This method returns a dictionary with all parameters that the corresponding "runner" expects. This method will be invoked once for every iteration during the race. In the example, we parameterize the query by randomly selecting a profession from a list.
* ``infinite``: This property helps Rally to determine whether to let the parameter source determine when a task should be finished (usually when ``infinite`` is ``False``) or whether the task properties (e.g. ``iterations`` or ``time-period``) determine when a task should be finished.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are missing the crucial information here about how the custom parameter source can trigger a task stop by raising a StopIteration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. I've addressed this in 06add96.

@dliappis
Copy link
Contributor

dliappis commented Sep 9, 2019

LGTM thanks!

Copy link
Contributor

@ebadyano ebadyano left a comment

Choose a reason for hiding this comment

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

LGTM

@danielmitterdorfer
Copy link
Member Author

Thank you both for the reviews! :)

@danielmitterdorfer danielmitterdorfer merged commit 8979d9c into elastic:master Sep 10, 2019
@danielmitterdorfer danielmitterdorfer deleted the task-completion-via-param-source branch September 10, 2019 05:14
danielmitterdorfer added a commit to danielmitterdorfer/rally that referenced this pull request Sep 10, 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
danielmitterdorfer added a commit that referenced this pull request Sep 10, 2019
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
Relates #768
novosibman pushed a commit to novosibman/rally that referenced this pull request Oct 2, 2019
There are multiple conditions how a task can be completed. Either the user
explicitly specifies a maximum number of iterations or a time-period or a task
is implicitly completed when the parameter source is exhausted (e.g. when
reading parameters from a file). In the latter case it is not necessary to
provide any indication when a task is complete because the parameter source can
determine it. However, so far this was required.

With this commit we introduce a change in the parameter source API so that it is
possible to determine this case and also provide proper progress indication.
Furthermore we adapt all parameter sources that come with Rally out of the box
and provide guidance for users how to change their own parameter sources.

Relates elastic#763
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
@danielmitterdorfer danielmitterdorfer added the breaking Non-backwards compatible change label Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Non-backwards compatible change 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.

None yet

3 participants