Polish: RampingVUs

[inancgumus]

Since my refactoring steps are getting bigger and bigger, I think it's better to create a new issue for the RampingVUs executor to track what's going on here.

It's also because there are and possibly will be multiple PRs related to this issue:

• Refactor ramping VUs executor #2155
• Add a new test for handleRemainingVUs
• Fix TODOs inside the code

[inancgumus]

Tests still pass when I remove the handleRemainingVUs method from the code. I tried to make a failing test, but there are no remaining VUs in either case: with the handleRemainingVUs method or without.

What should I test for? IDK whether there is a need for a test?

Any ideas are appreciated 🙏 @na-- @imiric @mstoykov @codebien @yorugac

[mstoykov]

As far as I can see this code is mostly useful if you have gracefulRampDown that needs to finish before the gracefulShutDown so something like

export const options = { 
  stages: [ 
    { target: 5, duration: "5s"},
    { target: 0, duration: "5s"}
  ]
}

In this case with the default gracefulRampDown/Shutdown of 30s and a default function that just blocks we should stop the 5th VU at 5s(first stage)+1s( from the second stage) + 30s(gracefulRampDown), the 4th will be +1s and so on. instead of without it in which case they should stop at 5s+5s+30s(gracefulShutDown) which mostly will also happen because the context used has the deadline of the stages durations + gracefulShutDown.

So I guess you now need to extend/remake one the tests to add a way to test something in this vain (possibly with no 5 VUs and not with 30s+ of a test ;) )

[inancgumus]

@mstoykov Is there a similar code that I can look at?

[mstoykov]

A more complete example:

import { sleep } from "k6";
export const options = {
	scenarios: {
		t : {
			executor :"ramping-vus",
			gracefulStop: "5s",
			gracefulRampdown: "3s", 
			stages: [
				{duration: "1s", target: 2},
				{duration: "4s", target: 0}
			]
		}
	}
}

export default function () {
	sleep(6.5);
	console.log(__VU, __ITER);
}

If the feature is working you should get

INFO[0007] 1 0                                           source=console

if it's not you will get

INFO[0007] 1 0                                           source=console
INFO[0008] 2 0                                           source=console

or both vus will finish their iteration while only the first one should have the time

[mstoykov]

There already is a test that tested something similar, just apparently not to the extent it needed.

Here is proposed-test-change.txt patch with some changes that now break when you remove the lines. I am not particularly certain this now doesn't miss something else so maybe it's better to be added as a separate test

[inancgumus]

Thanks!

I didn't get what this means :(

... not the extended needed

Btw, why do we need to handle the remaining VUs? It's because when I tested, the number of VUs became 0 at the end, with or without handleRemainingVUs.

[mstoykov]

I didn't get what this means :(

It means that either I sleep or my autocorrect is ... going wrong I meant to the extent it needed but might've either mistyped most of it or my autocorrect correcting too much 🤦

Btw, why do we need to handle the remaining VUs? It's because when I tested, the number of VUs became 0 at the end, with or without handleRemainingVUs.

while we will always stop the VU execution without it we won't stop them at the particular time they should be based on the gracefulRampdown but because the whole scenario has runner through it's max duration. You can see https://github.com/inancgumus/k6/blob/f953e94c95478ad73f8033d1492f39376ecad5f9/lib/executor/ramping_vus.go#L281-L325 for examples where this might turn out to be very important and also on more detailed explanation what gracefulRampdown does

[na--]

Closed by #2155, right?

[inancgumus]

Closed by #2155, right?

Yep!