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

Editing a running test in the Web UI with class-picker restarts user count back at 0 #2204

Closed
edmundlam opened this issue Sep 16, 2022 · 11 comments
Labels

Comments

@edmundlam
Copy link

Describe the bug

When I edit a running load test in the Web UI with class-picker active, it restarts the users back at 0 users and ramps up to the desired count.

This is different from the behaviour without --class-picker, where if I edit to go from 10 to 20 users, it will merely add 10 more users on top of the existing 10.

Checking the code in web.py, it seems that if class-picker is active, the code will always restart the runners.

def swarm() -> Response:
            assert request.method == "POST"

            [...]

            # Loading UserClasses & ShapeClasses if Locust is running with UserClass Picker
            if self.userclass_picker_is_active:

                [...]

                # Stopping Runners and setting user dispatchers to None so that they
                # are recreated with the appropriate UserClasses
                self._stop_runners()

I'm wondering if we can skip the restarting of the runners if the user classes requested in the POST /swarm request are the same as those currently running. Especially since the "Edit" form for a running task doesn't let you change the class of users!

If this is something that you would agree to, I would be willing to open a PR to contribute the change. Thanks!

Expected behavior

When using --class-picker, increasing from 10 to 20 will add 10 more users

Actual behavior

When using --class-picker, increasing from 10 to 20 users will start from 0 again, and ramp up to 20 users.

Steps to reproduce

  1. Run locust with --class-picker
  2. Start a test with X users
  3. Edit the test to set it to X + 3 users
  4. Problem is that users start back from 0, instead of staying at X and adding 3 users.

You can also run without --class-picker to see that the expected behaviour occurs when --class-picker is not set.

Environment

  • OS: MacOS
  • Python version: 3.9.7
  • Locust version: 2.12.0
  • Locust command line that you ran: locust -f tests/load/locustfile.py --config tests/load/locust.conf # with class-picker = true in the conf
  • Locust file contents (anonymized if necessary):
@edmundlam edmundlam added the bug label Sep 16, 2022
@cyberw
Copy link
Collaborator

cyberw commented Sep 16, 2022

@mikenester ?

@mikenester
Copy link
Contributor

There's definitely an error if you don't stop the runners, but off of the top of my head I don't remember what it is. I'll try to see if there's a workaround for it.

@edmundlam
Copy link
Author

If it helps, this was the monkeypatch I am using to acheive the expected behaviour for my usage. I haven't tested it thoroughly though.

diff --git a/locust/web.py b/locust/web.py
index de688030..452dbf85 100644
--- a/locust/web.py
+++ b/locust/web.py
@@ -174,6 +174,8 @@ class WebUI:
                         # Defaulting to using all available user classes
                         user_classes = self.environment.available_user_classes

+                restart_required = list(user_classes.values()) != self.environment.user_classes
+
                 self._update_user_classes(user_classes)

                 # Updating ShapeClass if specified in WebUI Form
@@ -185,7 +187,8 @@ class WebUI:

                 # Stopping Runners and setting user dispatchers to None so that they
                 # are recreated with the appropriate UserClasses
-                self._stop_runners()
+                if restart_required:
+                    self._stop_runners()

             parsed_options_dict = vars(environment.parsed_options) if environment.parsed_options else {}
             for key, value in request.form.items():

@mikenester
Copy link
Contributor

I have a fix here: #2207. It turns out that I didn't need to stop the runners, just set the user_dispatcher to None so that the runners recreate it with the appropriate users classes.

@edmundlam
Copy link
Author

Hello @mikenester thanks for the quick turnaround! However from my quick test it seems to still have the same behaviour.

I took the latest dev build locust==2.12.1.dev36 and verified that the code has the changes from #2207.

Then I ran a simple locust against my endpoint going from 3 to 10, 15 and back to 5 users. This is what I got:

image

Seems like setting the user_dispatcher to None still resets the users. Is that possible?

@mikenester
Copy link
Contributor

Ahh sorry, it was working on my end. I'll talk another look at it today.

@edmundlam
Copy link
Author

Thanks, let me know if you need any more details if you can't repro the same behaviour on your side 👍

@mikenester
Copy link
Contributor

Second fix here: #2210

Looks like I was wrong about

There's definitely an error if you don't stop the runners

I removed the step completely to check for errors and didn't see any. I'm a little nervous about it though, as I specifically remembering that some workflows would cause an error. I think it was:

  1. Run a test with multiple user classes
  2. Run a new test with only one of those user classes

I couldn't replicate any errors when testing this out, however.

@edmundlam
Copy link
Author

Hi @mikenester, PR looks good to me 👍

I removed the step completely to check for errors and didn't see any. I'm a little nervous about it though, as I specifically remembering that some workflows would cause an error. I think it was:

1. Run a test with multiple user classes

2. Run a new test with only one of those user classes

I couldn't replicate any errors when testing this out, however.

I just tested this workflow and it seems to be fine for me as well. Hopefully it doesn't cause any other issues.

Thanks again for the support!

@cyberw
Copy link
Collaborator

cyberw commented Sep 20, 2022

👍👍 @edmundlam please close this when you have tried it out using a dev release.

@edmundlam
Copy link
Author

Tested with locust==2.12.1.dev52 and its working perfectly. Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants