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

Added option to WorkGroup to limit the number of concurrently runned tasks #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

opengs
Copy link

@opengs opengs commented May 27, 2024

As a possible resolution of the problem presented in #59 I added option limit number of tasks in group.

Unfortunatelly, i had to add golang.org/x/sync v0.7.0 library, so now there is dependency.

@opengs
Copy link
Author

opengs commented May 27, 2024

Maybe there should be also non blocking option with group buffer

@alitto
Copy link
Owner

alitto commented May 28, 2024

Hey @opengs!
Thanks for submitting your feature request and this PR 🙂
I think that adding an option to limit the number of tasks of a given group that can be running concurrently at any given time is a great idea. This concept would be the equivalent of maxWorkers at the worker pool level. If that's the case, I would rename it as MaxWorkers() for consistency. This option would become the way of specifying the max number of workers that can be allocated to execute tasks belonging to this specific group.
I looked at this PR and noticed the current approach (using a semaphore inside the task fn) has the undesired side-effect of blocking workers that pick up tasks after the concurrency limit has been reached. This happens because each WorkerPool has a single shared task queue (bffered channel) and all running workers pick up tasks in the order they were submitted.
For instance, if you set up a WorkerPool with 4 worker, create a Group with the maxTasks option set to 2 and then submit 4 tasks to this group, tasks Nº3 and 4 will be picked up by a worker but their execution will pause until the semaphore is released by one of the first 2 tasks. Since each worker can only execute 1 task at any given time, this causes these 2 workers to block and stop processing other tasks in the queue.

I will look into this when I have some free time and try to come up with a proposal on how to implement this. I have some ideas to explore, like using tasks to implement "nested workers". This means Groups will behave like "sub-pools"/"nested pools", creating a recursive pattern which seems interesting.

Please let me know your thoughts and any ideas you may have. Also, it would be great if you can share more details on the specific use case where you saw the need for this feature, I'm curious to know more about it.
Thanks again!

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.

None yet

2 participants