[benchmarker] Add timeline artifact - #1664
Conversation
mickmis
left a comment
mickmis
left a comment
There was a problem hiding this comment.
Note: I've not actually reviewed the html templates. Looking at the screenshot outcome sounds good enough to me.
LGTM
| @@ -93,7 +93,7 @@ local shape = { | |||
| }, | |||
| }, | |||
| flight_execution: { | |||
| end_flight_after_start: '10s', | |||
| end_flight_after_start: '5s', | |||
There was a problem hiding this comment.
For my own understanding, are those parameters changes only cosmetic for the timeline to display better, or are they significant for something else? Notably in regard to the recent discussions about benchmarker vs locust behavior.
There was a problem hiding this comment.
These adjustments are a continued attempt to get to a "good" set of scenarios for representative load testing. The two aspects of "good" are that 1) the behavior of the load is representative of real life (e.g., it doesn't have an artificially short window for something to happen that could be fine taking longer in real life) and 2) it executes as quickly as possible (we don't want to wait many hours for a test run unless absolutely necessary).
The 10s -> 5s change for flight_execution.end_flight_after_start addresses the second issue by hopefully making each flight 5s faster without violating the first issue. The fixed_spacing addresses the first issue by making sure there's enough time to delete the operational intent without colliding with the start of the next flight. The uniform_random_spacing also addresses the first issue by attempting to more smoothly decorrelate the start times of virtual users in a group. I expect we'll continue to refine these values as we gather more information via better tools (this PR is a big one I think) and more measurements.
| return round(value / resolution) * resolution | ||
|
|
||
|
|
||
| def format_duration_shorthand(duration: float | datetime.timedelta) -> str: |
There was a problem hiding this comment.
nit: this package exists, to be considered if it's worth it or not if we will need more of that kind of human-readable display of values
There was a problem hiding this comment.
I do like not needing to maintain code ourselves, but it looks like that package does longhand representations -- these are short, with a particular logic to "significant units" (like significant figures) so I don't think human-readable is a substitute candidate in this case.
BenjaminPelletier
left a comment
BenjaminPelletier
left a comment
There was a problem hiding this comment.
Note: I've not actually reviewed the html templates. Looking at the screenshot outcome sounds good enough to me.
I agree reviewing the rendered HTML seems good enough in this case as the architecture of the HTML template should not substantively impact anyone since developers will not build upon it more and users will not see it.
| @@ -93,7 +93,7 @@ local shape = { | |||
| }, | |||
| }, | |||
| flight_execution: { | |||
| end_flight_after_start: '10s', | |||
| end_flight_after_start: '5s', | |||
There was a problem hiding this comment.
These adjustments are a continued attempt to get to a "good" set of scenarios for representative load testing. The two aspects of "good" are that 1) the behavior of the load is representative of real life (e.g., it doesn't have an artificially short window for something to happen that could be fine taking longer in real life) and 2) it executes as quickly as possible (we don't want to wait many hours for a test run unless absolutely necessary).
The 10s -> 5s change for flight_execution.end_flight_after_start addresses the second issue by hopefully making each flight 5s faster without violating the first issue. The fixed_spacing addresses the first issue by making sure there's enough time to delete the operational intent without colliding with the start of the next flight. The uniform_random_spacing also addresses the first issue by attempting to more smoothly decorrelate the start times of virtual users in a group. I expect we'll continue to refine these values as we gather more information via better tools (this PR is a big one I think) and more measurements.
| return round(value / resolution) * resolution | ||
|
|
||
|
|
||
| def format_duration_shorthand(duration: float | datetime.timedelta) -> str: |
There was a problem hiding this comment.
I do like not needing to maintain code ourselves, but it looks like that package does longhand representations -- these are short, with a particular logic to "significant units" (like significant figures) so I don't think human-readable is a substitute candidate in this case.
2 participants
This PR adds a timeline artifact to benchmarker which shows all the operations of interest happening in parallel visually:
Example generated from the changes to single_s2_cell.jsonnet:
timeline.zip
I specified the configuration and "product spec", but nearly all the implementation is via Gemini. I'm not too concerned about perfect architecture as I expect this to be "dead end" code only used for generating this artifact and not as a building block for anything else in the future.