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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mstoykov
Copy link
Collaborator

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
@na-- na-- self-requested a review June 15, 2022 10:43
lib/executor/ramping_vus.go Outdated Show resolved Hide resolved
lib/executor/ramping_vus.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

🤞

@@ -244,7 +250,7 @@ func (vlvc RampingVUsConfig) getRawExecutionSteps(et *lib.ExecutionTuple, zeroEn
}

if zeroEnd {
steps = append(steps, lib.ExecutionStep{TimeOffset: timeTillEnd, PlannedVUs: 0})
addStep(timeTillEnd, 0)
Copy link
Collaborator 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.

None yet

5 participants