Adding ability to generate any custom load shape with LoadTestShape class #1505

max-rocket-internet · 2020-08-06T13:58:43Z

Resolves #1432

Adding a base LoadTestShape class for users to inherit in order to generate any user_count/hatch at 1 second resolution.

The the locustfile will be searched for a LoadTestShape class and used it if present
If present, this will run as a greenlet on the master and will continuously edit/update and optionally stop a running load test
Added 3 examples to show how it could be used, including step-load test like Visual Studio, stages like k6 and a completely customised double wave.
User and hatch rate in the UI is effectively ignored if a LoadTestShape class is present. This is stated in a log message.
Locust command will exit with error if a LoadTestShape is present and a conflicting option is also specified as argument (users, hatch-rate or step-load)
Create documentation

Tests:

Update 4 tests to ensure the LoadTestShape class is not in user_classes
A full distributed test with master + 3 workers scaling up, down and stop
Scale up and scale down tests for TestMasterRunner

Initial testing looks good 🚀

Comments from @cyberw

I think the shape should be possible to have in a separate file

We'll need a couple really of good unit tests for this

Please give me suggestions if we need more tests as I'm not sure. This addition is simply editing a running test so much of this functionality should be covered already.

make sure there is no error for parameters (-u, -t etc) not being specified when there is a shape.

It will exit with an error.

correctly applying stop_timeout when ramping down
where new slaves are added during the test

The current behaviour here won't change?

Comments from @heyman

Unfortunately the stepload implementation isn't very good and I think it should be removed

We can replace the stepload code with the more general approach in this PR but also this PR is quite similar to the stepload implementation. Please clarify.

codecov · 2020-08-06T14:01:35Z

Codecov Report

Merging #1505 into master will increase coverage by 0.05%.
The diff coverage is 66.07%.

@@            Coverage Diff             @@
##           master    #1505      +/-   ##
==========================================
+ Coverage   81.45%   81.50%   +0.05%     
==========================================
  Files          27       28       +1     
  Lines        2410     2460      +50     
  Branches      372      380       +8     
==========================================
+ Hits         1963     2005      +42     
- Misses        356      359       +3     
- Partials       91       96       +5

Impacted Files	Coverage Δ
locust/web.py	`90.05% <0.00%> (-1.97%)`	⬇️
locust/main.py	`20.09% <50.00%> (+1.43%)`	⬆️
locust/runners.py	`83.23% <72.00%> (+1.36%)`	⬆️
locust/shape.py	`80.00% <80.00%> (ø)`
locust/__init__.py	`100.00% <100.00%> (ø)`
locust/env.py	`96.72% <100.00%> (+0.16%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19b2dc0...93a41ee. Read the comment docs.

cyberw · 2020-08-06T18:03:42Z

locust/test/test_runners.py

+            for _, msg in server.outbox:
+                if msg.data:
+                    num_users += msg.data["num_users"]
+            self.assertEqual(3, num_users, "Total number of users in second stage of shape test is not 3: %i" % num_users)


Maybe also test ramp down?

Good idea. I'll add another test. I had a problem that the test only runs for 5 seconds and is then stopped, regardless of what I set. I couldn't see why. Is that correct?

OK now there are 2 tests, one for scaling up and one for scaling down. Ideally they would be in the same test but that 5s issue makes it too tight.

Hmm... I didnt know there was a 5 second limit and cant really understand where it comes from :)

cyberw · 2020-08-06T18:05:57Z

locust/runners.py

@@ -71,10 +73,13 @@ def on_request_failure(request_type, name, response_time, response_length, excep
        def on_hatch_complete(user_count):
            self.state = STATE_RUNNING
            if environment.reset_stats:
-                logger.info("Resetting stats\n")
+                logger.info("Resetting stats")


is this change intentional?

Yes, I thought the \n was a mistake. It's intentional?

I'll revert it anyway

Tbh I dont know :)

cyberw · 2020-08-06T18:08:45Z

locust/runners.py

-
+
+        if self.environment.shape_class:
+            logger.debug("Starting with shape class")


Looks like you are kind of logging this twice? (here and on line 286)

True. I'll remove this one.

cyberw · 2020-08-07T19:48:37Z

examples/custom_shape/stages.py

+            duration -- When this many seconds pass the test is advanced to the next stage
+            users -- Total user count
+            hatch_rate -- Hatch rate
+            stop -- A boolean that can stop that test at a specific stage


Maybe a None entry could be better used to signal the end of the test? Because you'll never need to do (x, y, True) for any other values of x or y than zero, right?

Well we need to return at least (Int, Int) to edit the running test so you are saying to stop the test return (None, None)? Or (Int, Int, None)? Or something else?

I meant that tick() should do return None instead of return (0, 0, True) (which of course requires modifications in runners.py on line 297 as wel)

But, I also think that stop_at_end is an unnecessary complication. If you want to run your test forever, cant you just set the last step to use 'duration': 9999999 ?

But, I also think that stop_at_end is an unnecessary complication

This is just an example of what I imagine someone might want to use or see before they write their own. But I can remove stop_at_end it to make it simpler, no worries.

cant you just set the last step to use 'duration': 9999999

I would say no, that's not very precise. What if you forget it's running and it gets to 10000000 seconds😃

Just to reiterate, these are examples. I imagine they would be in the docs or just here for people to find. They aren't to be copy/pasted and used as they are. I just imagined them to help a beginner to see how this class could be used 🙂

Sure. Lets not spend too much time on them. On the other hand, if they are just examples then it makes sense to keep them very simplified (and let our users worry about edge cases like how to make it run forever etc).

cyberw · 2020-08-07T19:54:15Z

locust/shape.py

+        """
+        Calculates run time in seconds of the load test
+        """
+        return round(time.monotonic() - self.start_time)


I'm not sure a rounded time is a good thing to expose. This can give people the illusion that tick is called every second (which cannot be guaranteed because of gevent - the only thing your code guarantees is that it will not be called with less than one second between calls)

cyberw · 2020-08-07T20:00:26Z

examples/custom_shape/double_wave.py

+    Keyword arguments:
+
+        min_users -- minimum users
+        peak_one -- first peak size


peak_one sounds to me like "the number of users at the first peak", which could be misleading.

Not sure what would be a better name though.

I just threw this together to show how you could generate something totally custom and not related to "stages" or "step-load". I'll rework the wording here and maybe improve the maths.

cyberw · 2020-08-07T20:06:17Z

examples/custom_shape/double_wave.py

+        amplitude -- size of the test
+        time_limit -- total length of test
+    """
+    def __init__(self, min_users=20, peak_one=1, peak_two=1.5, amplitude=150, time_limit=600):


This looks a little weird to me.

It's not as if this constructor can be called from somewhere where these parameters are ever set, right?

Probably all these variables should be class level and then you can get rid of the parameters for the constructor

True. I'll fix.

cyberw · 2020-08-07T20:07:48Z

examples/custom_shape/stages.py

+                {'duration': 120, 'users': 70, 'hatch_rate': 10},
+                {'duration': 180, 'users': 100, 'hatch_rate': 10},
+                {'duration': 220, 'users': 10, 'hatch_rate': 10},
+                {'duration': 230, 'users': 5, 'hatch_rate': 10},


Include a stage with ramp down just to show that it can be done, I think that would be nice!
(but maybe reduce the number of stages, it doesnt add much information)

This does ramp down, see users key, it goes up and then down 🙂

But OK, I'll reduce number of stages here.

Ah, my comment was about having so many stages, not about the very last stage :)

max-rocket-internet · 2020-08-10T12:26:16Z

@cyberw I've addressed all your comments

max-rocket-internet · 2020-08-10T21:57:55Z

locust/shape.py

+        """
+        run_time = self.get_run_time()
+
+        return (0, 0, True)


@cyberw comments about what to return should go here

I meant that tick() should do return None instead of return (0, 0, True)

Is that consistent? Wouldn't something like (Int, Int), (None, None) or () be better?

I dont know what you mean by (Int, Int) - how would that be used indicate the end of the test?

I think None is the most consistent of the options, also it requires the least amount of code and thought from the user.

It is not a ramp down to null users using ramping down at a rate of null per second, so your second option doesnt make sense to me.

I guess we could do "anything that is "falsy" (to allow (), None and False), if that sounds better to you.

I meant trying to keep the type that is returned consistent in all cases. It returns a tuple (of 2 integers normally (Int, Int)) so if you we want to be consistent then it should always return a tuple, perhaps sometimes an empty one, ().

But I know this is a contentious topic in Python, especially for people coming from other languages who find it a bit too loose.

Ultimately I don't care enough to write more comments about this so I'll just change it to return (Int, Int) or None 😃

max-rocket-internet · 2020-08-12T12:27:08Z

@cyberw please re-review and when you're happy I'll work on docs

cyberw · 2020-08-12T14:56:28Z

locust/runners.py

@@ -293,13 +293,13 @@ def shape_worker(self):
        logger.info("Shape worker starting")
        while self.state == STATE_INIT or self.state == STATE_HATCHING or self.state == STATE_RUNNING:
            new_state = self.environment.shape_class.tick()
-            user_count, hatch_rate, stop_test = new_state
-            if stop_test:
+            if new_state == None:


use is None (in this particular case, I'm not sure it makes a functional difference, but it is still nicer :)

As you wish ✅

cyberw · 2020-08-12T16:01:26Z

Looks great now. One question. How does it work when running web UI?

@heyman I'll probably merge this soon. Last chance to give your view. Step two could be to remove the step-load thingy if you agree this is better.

max-rocket-internet · 2020-08-12T16:06:47Z

I still need to write docs and this is not a quick task IMO.

How does it work when running web UI?

Like I said in my opening comment, basically if a load test is started via the UI or API, the hatch rate and users is simply ignored. I think that's fair, if you're using a custom shape you will be a pretty advanced user anyway. I was thinking we could log a warning? What do you think?

cyberw · 2020-08-12T16:46:50Z

I think logging a warning is fine (we should change that for -u/-r/-t parameters to make them behave that way as well tbh, but that may be a later thing)

max-rocket-internet · 2020-08-14T13:12:25Z

OK I've included a few words in the log message to indicate the user_count/hatch_rate are ignored. I've also disabled editing a running shape test as this doesn't make sense.

I'll work on docs now.

…te being ignored

max-rocket-internet · 2020-08-14T14:40:06Z

OK I wrote a nice doc. Personally I think this feature deserves a mention in docs/what-is-locust.rst? But I'm obviously biased 😅

Anything else @cyberw?

max-rocket-internet · 2020-08-14T14:50:37Z

Do I have to generate the API docs myself? Or is that done in CI or something?

$ make build_docs
sphinx-build -b html docs/ docs/_build/
Running Sphinx v2.4.3
Running `locust --help` command and storing output in /Users/max.williams/git/locust/docs/cli-help-output.txt
Generating RST table for Locust environment variables and storing in /Users/max.williams/git/locust/docs/config-options.rst

Extension error:
Could not import extension sphinx_search.extension (exception: No module named 'sphinx_search')
make: *** [build_docs] Error 2

cyberw · 2020-08-14T15:10:24Z

While you are adding the warning about ignoring -u/... when running with web ui, can you do it for non-custom-load-shapes as well? I've been annoyed by that many times + it is good to be consistent. Bonus points if you prefill the web ui form with the values from parameters instead of warning :)

Api docs are generated in CI (but if you install the readthedocs-sphinx-search package then you should be able to build them locally as well)

cyberw · 2020-08-14T15:14:47Z

Regarding the docs, I think you can add it the same way as we did step load (a mention in the menu and possibly also a link in a suitable place, but keep it in a separate rst file). And then after a while, we remove step load :P Or maybe re-implement it using this method.

max-rocket-internet · 2020-08-14T17:23:33Z

While you are adding the warning about ignoring -u/... when running with web ui, can you do it for non-custom-load-shapes as well?

There's not really a place for me to log warning if user/hatch is passed because these are ALWAYS passed. In master, locust will throw an exception or error if you don't set user/hatch when starting a test.

can you do it for non-custom-load-shapes as well? I've been annoyed by that many times + it is good to be consistent

I'm happy to add something but I'm not 100% sure what you mean. You mean a combination of command arguments and the UI? Or?

Here's what I have now in this PR, when running locust from command line:

    if shape_class and (options.num_users or options.hatch_rate or options.step_load):
        logger.error("The specified locustfile contains a shape class but a conflicting argument was specified: users, hatch-rate or step-load")
        sys.exit(1)

If you start a load test from the UI or API and there is a shape class in use then user/hatch is ignored and this is logged:

logger.info("Shape test starting. User count and hatch rate are ignored for this type of load test")

Editing a running load test with a shape class nothing but will log:

logger.info("There is an ongoing shape test running. Editing is disabled")

max-rocket-internet · 2020-08-14T17:29:27Z

I've been annoyed by that many times + it is good to be consistent. Bonus points if you prefill the web ui form with the values from parameters instead of warning :)

If you create an new issue (it sounds separate) then I'll happily address it in a new PR 🙂

cyberw · 2020-08-15T11:44:47Z

Looking really good now. Is it worth adding a test case for master/worker setup as well? (I’m on mobile so if there already is one then maybe i missed it :)

max-rocket-internet · 2020-08-17T09:03:21Z

Is it worth adding a test case for master/worker setup as well?

OK @cyberw, done, with master + 3 workers scaling up, down and stop.

Are we ready? 😃 🙏

cyberw · 2020-08-17T10:57:35Z

Looks amazing. One last question: what will happen if additional workers connect during the test? Will new users launch on the newly connected workers?

Tbh I'm not sure that is a use case we need to provide full support for, but some people argued differently (when we were discussing global rps wait time I think). If it should work then maybe add it to the test? (adding a worker and then ramping up, making sure the users become balanced after ramp up)

max-rocket-internet · 2020-08-17T11:19:52Z

what will happen if additional workers connect during the test? Will new users launch on the newly connected workers?

This behaviour is unchanged by this PR. Users are rebalanced across all workers when new workers connect. Does a test of that functionality have a relation to this PR? There is already the test_rebalance_locust_users_on_worker_connect test

cyberw · 2020-08-17T12:08:46Z

I was just thinking those two features might be worth testing together. Maybe not. Merging now! Really cool stuff!

max-rocket-internet · 2020-08-17T16:20:27Z

@cyberw when is a new release likely to be made?

cyberw · 2020-08-17T17:07:22Z

Tonight if you’re lucky, tomorrow if you are not :)

cyberw · 2020-08-17T22:19:43Z

You weren't lucky :(

Also, I think I'll want to hold of for some more merges (1468 in particular seems to be very close)

cyberw · 2020-08-17T22:33:27Z

Also, can you make a PR that greys out the numbers that cant be edited in the UI when running with a shape?

Also, I noticed something weird. What is going on here:

locust/locust/shape.py

Line 34 in 62fa37b

run_time = self.get_run_time()

the variable run_time is never used...

max-rocket-internet · 2020-08-18T07:29:00Z

Also, can you make a PR that greys out the numbers that cant be edited in the UI when running with a shape?

I can.

the variable run_time is never used...

That is intentional. The user has to override tick() anyway and this was to show that they most likely want to calculate the run time and that this is how you do it. All 3 examples have the same line (but it's used). I can remove it? Or add a comment?

cyberw · 2020-08-18T07:35:10Z

👍

I think it makes sense to remove it, feels like it adds about as much confusion as it removes :) Also, pylance complains about it.

max-rocket-internet · 2020-08-18T08:41:08Z

OK: #1524

max-rocket-internet mentioned this pull request Aug 6, 2020

Support for completely custom load pattern / shape #1432

Closed

cyberw reviewed Aug 6, 2020

View reviewed changes

cyberw reviewed Aug 7, 2020

View reviewed changes

max-rocket-internet commented Aug 10, 2020

View reviewed changes

cyberw reviewed Aug 12, 2020

View reviewed changes

max-rocket-internet added 12 commits August 14, 2020 16:32

Initial commit with shaper

673db60

renaming to shape, add some logging, examples

fa4c70d

fix existing tests by adjusting param order

1782959

add test to ensure LoadTestShape is not in user_classes

b5e9fdc

add a test

428c67f

exit with error if a conflicting argument is specified

c53b625

Make formatting and style consistent

956a629

Remove double log

efcef24

Add test for scaling down also

bf932ae

Revert removing \n

f593eb2

rewrite DoubleWave example

1e20f88

simplify stages example

d84d0c0

max-rocket-internet added 5 commits August 14, 2020 16:33

y u round, no round

37271ec

Simplify examples, return None to stop test

f921aea

is None? Yes is None.

e32ec31

disable editing running test. Print message about user_count/hatch_ra…

36738ad

…te being ignored

Adding doc

ecad2f7

update docs, change one word in log message

e3ec440

Add full test with workers and master

93a41ee

cyberw merged commit ecee93a into locustio:master Aug 17, 2020

max-rocket-internet deleted the loadtest_shaper branch August 17, 2020 16:19



		if self.environment.shape_class:
		logger.debug("Starting with shape class")

Adding ability to generate any custom load shape with LoadTestShape class #1505

Adding ability to generate any custom load shape with LoadTestShape class #1505

Conversation

max-rocket-internet commented Aug 6, 2020 • edited Loading

codecov bot commented Aug 6, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw Aug 10, 2020 • edited Loading

Choose a reason for hiding this comment

max-rocket-internet Aug 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-rocket-internet commented Aug 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-rocket-internet commented Aug 12, 2020

cyberw Aug 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberw commented Aug 12, 2020

max-rocket-internet commented Aug 12, 2020

cyberw commented Aug 12, 2020

max-rocket-internet commented Aug 14, 2020

max-rocket-internet commented Aug 14, 2020

max-rocket-internet commented Aug 14, 2020

cyberw commented Aug 14, 2020 • edited Loading

cyberw commented Aug 14, 2020 • edited Loading

max-rocket-internet commented Aug 14, 2020

max-rocket-internet commented Aug 14, 2020

cyberw commented Aug 15, 2020 • edited Loading

max-rocket-internet commented Aug 17, 2020

cyberw commented Aug 17, 2020 • edited Loading

max-rocket-internet commented Aug 17, 2020

cyberw commented Aug 17, 2020 • edited Loading

max-rocket-internet commented Aug 17, 2020

cyberw commented Aug 17, 2020

cyberw commented Aug 17, 2020

cyberw commented Aug 17, 2020 • edited Loading

max-rocket-internet commented Aug 18, 2020

cyberw commented Aug 18, 2020

max-rocket-internet commented Aug 18, 2020

max-rocket-internet commented Aug 6, 2020 •

edited

Loading

codecov bot commented Aug 6, 2020 •

edited

Loading

cyberw Aug 7, 2020 •

edited

Loading

cyberw Aug 10, 2020 •

edited

Loading

max-rocket-internet Aug 10, 2020 •

edited

Loading

cyberw Aug 7, 2020 •

edited

Loading

cyberw Aug 7, 2020 •

edited

Loading

cyberw Aug 7, 2020 •

edited

Loading

cyberw Aug 7, 2020 •

edited

Loading

cyberw Aug 12, 2020 •

edited

Loading

cyberw commented Aug 14, 2020 •

edited

Loading

cyberw commented Aug 14, 2020 •

edited

Loading

cyberw commented Aug 15, 2020 •

edited

Loading

cyberw commented Aug 17, 2020 •

edited

Loading

cyberw commented Aug 17, 2020 •

edited

Loading

cyberw commented Aug 17, 2020 •

edited

Loading