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

Handle task weights efficiently in TaskSets #2820

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

Conversation

bakhtos
Copy link
Contributor

@bakhtos bakhtos commented Jul 29, 2024

Fixes #2651

After realising that my initial idea of storing a dictionary of task weights in TaskSet would not work due to incompatibility with SequentialTaskSet and the rest of the code (many things in the codebase assume tasks is a list, refactoring which will be too much work), I started from scratch compared to #2741 and here are the changes:

  • TaskSet and SequentialTaskSet now use the same metaclass, which uses the same get_tasks_from_base_classes method
    • SequentialTaskSet is moved to the task.py, since keeping it in a separate file is no longer sensible
    • get_tasks_from_base_classes method will process all tasks in order they are declared and put them in a list (only one of each), adding a locust_task_weight attribute with the corresponding weight if it is missing (which now all tasks are thus guaranteed to have)
    • TaskSet and SequentialTaskSet will process this list of tasks as they need, adding necessary data structures (in new _setup_tasks() method, see below
      • TaskSet will calculate the cumulative weights necessary for random.choices
      • SequentialTaskSet will expand the tasks list into a list where tasks are duplicated according to the weight and make a cycle out of it
  • @task decorator can now receive also a float as a weight

To facilitate these changes, also inheritance of classes is changed:

  • There is now AbstractTaskSet class, which keeps most of the functionality that was previously in the TaskSet (init, run, wait_time etc); TaskSet, SequentialTaskSet and DefaultTaskSet all inherit this class
    • It defines two abstract methods _setup_tasks() and get_next_task() which concrete classes need to implement
      • _setup_tasks() is necessary since the original tasks attribute can be filtered using tags after class instantiation, so the actual preparation of additional data structures needs to happen later; this method is called by run() (also, some tests needed to call it explicitly since they check some things without calling run(), but this should not be a problem
      • get_next_task() is the method that provides the next task to run(). Arguably, this is where the specialized logic of a new TaskSet class would be implemented
        • To make sure concrete classes can focus on their unique functionality, checking whether tasks is empty is now the responsibility of run() during its start. It is, after all, only necessary to do once and not on every call to get_next_task(). If there are no tasks, LocustError is raised. It is also raised by get_tasks_from_base_classes if tasks is not a dict or a list. Tests are also modified to check for this specific error rather than Exception.

Test are modified to account for these changes.

I think this refactoring makes it easier to introduce new specialized Task Sets, since there is now a unified data model/interface that they should accommodate.

Copy link

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale Issue had no activity. Might still be worth fixing, but dont expect someone else to fix it label Sep 30, 2024
Copy link

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Oct 11, 2024
@cyberw cyberw reopened this Oct 11, 2024
@cyberw cyberw removed the stale Issue had no activity. Might still be worth fixing, but dont expect someone else to fix it label Oct 11, 2024
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.

Use random.choices when selecting a task instead of random.choice
2 participants