Skip to content

Fix max duration calculations with multiple 0 vus stages at end#2573

Open
mstoykov wants to merge 4 commits intomasterfrom
moreRampingVUsFixes
Open

Fix max duration calculations with multiple 0 vus stages at end#2573
mstoykov wants to merge 4 commits intomasterfrom
moreRampingVUsFixes

Conversation

@mstoykov
Copy link
Copy Markdown
Contributor

In b725f03 we fixed the calculation of the stages, unfortunately we
didn't notice that the max duration for the test was wrongly calculated.

This also tries to fix cases where graceful* will extend the test more
then needed. Previously the code did not take into account that multiple
0 VUs stages can mean that the VUs are long ago stopped before
gracefulStop (for example) needs to be taken into account.

In b725f03 we fixed the calculation of the stages, unfortunately we
didn't notice that the max duration for the test was wrongly calculated.

This also tries to fix cases where graceful* will extend the test more
then needed. Previously the code did not take into account that multiple
0 VUs stages can mean that the VUs are long ago stopped before
gracefulStop (for example) needs to be taken into account.
@mstoykov mstoykov marked this pull request as ready for review June 15, 2022 10:13
@mstoykov mstoykov added this to the v0.39.0 milestone Jun 15, 2022
@github-actions github-actions Bot requested review from olegbespalov and oleiade June 15, 2022 10:14
@na-- na-- self-requested a review June 15, 2022 10:43
Comment thread lib/executor/ramping_vus.go Outdated
Comment thread lib/executor/ramping_vus.go Outdated
Copy link
Copy Markdown
Contributor

@olegbespalov olegbespalov left a comment

Choose a reason for hiding this comment

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

🤞


if zeroEnd {
steps = append(steps, lib.ExecutionStep{TimeOffset: timeTillEnd, PlannedVUs: 0})
addStep(timeTillEnd, 0)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looking at this again ... maybe I should just check that plannedVUs and timeOffset are different and add it only then 🤔

On the other hand we care to have a zero end and addStep will check for that it just might not be at the timeTillEnd offset 🤔 .... or maybe even that isn't possible with the changes above

@na-- na-- modified the milestones: v0.40.0, v0.41.0 Aug 15, 2022
@na-- na-- modified the milestones: v0.41.0, TBD Oct 28, 2022
@na-- na-- removed their request for review February 23, 2023 08:29
@codebien codebien removed this from the TBD milestone Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants