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

Bounbed task queues #91

Open
korotin opened this issue Dec 3, 2024 · 8 comments
Open

Bounbed task queues #91

korotin opened this issue Dec 3, 2024 · 8 comments

Comments

@korotin
Copy link
Collaborator

korotin commented Dec 3, 2024

V2's task queues are unbounded by default, but in some cases it may be useful to have them bounded. As far as i can see there's no option to do that right now.

Was that decision intentional? I mean are there any design limitations or conceptual thoughts that make such option unwanted or even impossible?

@alitto
Copy link
Owner

alitto commented Dec 3, 2024

Hey @korotin!
Initially I thought it wouldn't be necessary but after releasing v2 I came across different use cases where it would be really convenient to have bounded task queues so I have changed my mind around this. It's definitely on my list of things to add to pond but haven't had enough time to sit and work on it just yet.
I think the straightforward addition would be to have a new pool option called QueueSize or Capacity, that can be configured when creating a pool:

pool := pond.NewPool(10, pond.WithCapacity(1000))

Any thoughts on this? I'm still wondering what would be the best name for this option.

@korotin
Copy link
Collaborator Author

korotin commented Dec 3, 2024

Hey @alitto ! It's good to know that we're on the same page here.

I vote for QueueSize. It is understandable and straightforward to me while Capacity seems a little bit ambiguous. It is unclear capacity of what we're setting with that option.

On the other hand since you've called your new task queues unbounded, maybe play around that? I mean something like pond.BoundWith(1000).

@alitto
Copy link
Owner

alitto commented Dec 4, 2024

Cool, QueueSize seems like a good candidate. Another option may be QueueCapacity i guess. In v1 this concept was called capacity but i recognize it wasn't very clear.
Calling the option fn pond.BoundWith sounds very fluent, but I would prefer to keep it consistent with the other options (pond.WithQueueSize), to make it more predictable.
The other aspect I would need to define is the behavior of the pool.Submit method when the queue is full. I guess it should block the caller goroutine until there's room in the queue (before returning the pond.Task future).
And if we have this option, we may need to add a non-blocking alternative to submit tasks, maybe pond.TrySubmit() 🤔

@korotin
Copy link
Collaborator Author

korotin commented Dec 4, 2024

I completely agree with most of what you said, but I'm not 100% sure about the non-blocking scenario. Here's my concerns:

  1. We would have to add not one but three methods to Pond: TrySubmit, TrySubmitErr and TryGo which may be overwhelming for public interface and less intuitive.
  2. With default configuration (e.g. when task queue is unbounded) all these methods would be meaningless since they'd be equivalent to their non-Try siblings.

At the same time I do understand that cases when you don't want to wait if tasks cannot be added are completely legitimate and I don't see any other ways (at least good ones) to implement the non-blocking behavior.

@alitto
Copy link
Owner

alitto commented Dec 5, 2024

You have a great point, adding all those Try* methods increases the API surface unnecessarily, it can be confusing.
How about we expose a new pool option to control the behavior of the Submit method when the queue is full. Something like pond.NonBlocking, which indicates caller goroutines will not block when the queue is full and an error is returned (e.g. ErrQueueFull). Of course, this flag would only be honored when the pool is bounded (QueueSize is specified). By default, the queue will block caller gouroutines.
Putting it all together, this is how you would create a blocking bounded pool:

blockingBoundedPool := pond.NewPool(10, pond.WithQueueSize(10))

And to create a non-blocking bounded pool:

nonBlockingBoundedPool := pond.NewPool(10, pond.WithQueueSize(10), pond.NonBlocking)

What do you think?

@korotin
Copy link
Collaborator Author

korotin commented Dec 5, 2024

Actually I came up with exactly the same idea yesterday but wasn't sure if it's good enough :)

My main concern here is that such option would change global behavior which is non-granular and may be unexpected.

What do you think about expanding definitions of Submit-like functions to something like Submit(fn func(...), opts ...pond.SubmitOptionFn), to be able to submit tasks like this:

pool.Submit(func() { }, pond.NonBlocking)

With such expansion we would keep granularity and obviousness of Try* methods without polluting public API, but I do realise though that such change may seem like over-engineering for the kind of problem we're trying to solve.

@alitto
Copy link
Owner

alitto commented Dec 5, 2024

I really like the idea of having options on Submit methods. I think it works for pond.NonBlocking and it may even be a good idea to support pond.WithContext(ctx) as a submit-level option too, for use cases where you want to abort the task based on a context other than the pool context 🤔
We could also have pond.WithTimeout(timeout) to abort execution if timeout expires, and pond.WithDeadline(deadline) too.
However, I think having the pond.NonBlocking option at the pool-level makes a lot of sense, since most use cases I've seen want to have consistent behavior across tasks.

@korotin
Copy link
Collaborator Author

korotin commented Dec 5, 2024

Well, we may have pond.NonBlocking for both pool-level and task-level configurations, if that suits you :)

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

No branches or pull requests

2 participants