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

Variable-looping-vus/stages is a start/stopping instances a little flaky #1473

Closed
mstoykov opened this issue May 28, 2020 · 1 comment
Closed

Comments

@mstoykov
Copy link
Collaborator

It seems like sometimes variable looping vus will gracefully stop a VU while ramping up and then just start it again. After some additional logs, it is easy to spot
image
As is seen rawStep goes a little faster sometimes and gets 2-3 steps ahead of gracefulLimit and when we get a gracefulLimit that is basically for some steps ago and we stop a VU or two... gracefully :D.

This (at least for me) only happens if the machine is under load which is not all that hard especially when ramping up so ...
The code in question reads from two different channels that get events every time something needs to happen.
https://github.com/loadimpact/k6/blob/83d0fcf2ecf44db27275cdf7ea887c05edf9bd06/lib/executor/variable_looping_vus.go#L638-L673
I do think it will be much simpler and easier if we zip (combine) the two giving gracefulLimit the preference and just go through the new slice ...

This also will definitely be more performant, not that 2 goroutines and 2 channels are too much ... but we can skip them :D

@mstoykov mstoykov changed the title Variable loopign vus/stages is a start/stopping instances a little flaky Variable-looping-vus/stages is a start/stopping instances a little flaky Jun 2, 2020
mstoykov added a commit that referenced this issue Jun 9, 2020
Also fix VLV not respecting gracefulStop since 270fd91, add test for it
and try to make some other VLV tests more stable.

The original code (pre 270fd91) did go through all the raw steps
(ramp up/down) and the graceful stop ones concurrently and when the raw
steps ended it will return from Run, but will start a goroutine to go
through the remainign graceful stop ones. The most important of which (
at least in my current understanding) being the last one which is the
actuall gracefulStop end which will make all VUs stop. The reason why
this actually worked is that it also waiting on all activeVUs to have
ended being active, being returned in the new terminology, before it
actually cancels the context for all of them. So if the VUs manage to
end iterations in this time the executor will end earlier if not the
gracefuStop will make them.

270fd91 broke the returning of VUs which lead to the executor never
returning and moving the cancel "before" the waiting for activeVUs fixed
that at the cost of gracefulStop not being taken into account, but with
no tests, nobody noticed.

Here I basically revert that especially because vuHandle now returns VUs
when stopped.

fixes #1473
@na-- na-- added this to the v0.27.0 milestone Jun 9, 2020
mstoykov added a commit that referenced this issue Jun 9, 2020
Also fix VLV not respecting gracefulStop since 270fd91, add test for it
and try to make some other VLV tests more stable.

The original code (pre 270fd91) did go through all the raw steps
(ramp up/down) and the graceful stop ones concurrently and when the raw
steps ended it will return from Run, but will start a goroutine to go
through the remainign graceful stop ones. The most important of which (
at least in my current understanding) being the last one which is the
actuall gracefulStop end which will make all VUs stop. The reason why
this actually worked is that it also waiting on all activeVUs to have
ended being active, being returned in the new terminology, before it
actually cancels the context for all of them. So if the VUs manage to
end iterations in this time the executor will end earlier if not the
gracefuStop will make them.

270fd91 broke the returning of VUs which lead to the executor never
returning and moving the cancel "before" the waiting for activeVUs fixed
that at the cost of gracefulStop not being taken into account, but with
no tests, nobody noticed.

Here I basically revert that especially because vuHandle now returns VUs
when stopped.

fixes #1473
@na-- na-- modified the milestones: v0.27.0, v0.28.0 Jun 10, 2020
mstoykov added a commit that referenced this issue Jun 10, 2020
Also fix VLV not respecting gracefulStop since 270fd91, add test for it
and try to make some other VLV tests more stable.

The original code (pre 270fd91) did go through all the raw steps
(ramp up/down) and the graceful stop ones concurrently and when the raw
steps ended it will return from Run, but will start a goroutine to go
through the remainign graceful stop ones. The most important of which (
at least in my current understanding) being the last one which is the
actuall gracefulStop end which will make all VUs stop. The reason why
this actually worked is that it also waiting on all activeVUs to have
ended being active, being returned in the new terminology, before it
actually cancels the context for all of them. So if the VUs manage to
end iterations in this time the executor will end earlier if not the
gracefuStop will make them.

270fd91 broke the returning of VUs which lead to the executor never
returning and moving the cancel "before" the waiting for activeVUs fixed
that at the cost of gracefulStop not being taken into account, but with
no tests, nobody noticed.

Here I basically revert that especially because vuHandle now returns VUs
when stopped.

fixes #1473

Co-authored-by: na-- <n@andreev.sh>
@na--
Copy link
Member

na-- commented Jun 24, 2020

Resolved by #1496 and #1506

@na-- na-- closed this as completed Jun 24, 2020
@mstoykov mstoykov modified the milestones: v0.28.0, v0.27.0 Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants