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

Improve fairness of TaskExecutorDistributor #45

Closed
jentfoo opened this issue Oct 22, 2013 · 3 comments
Closed

Improve fairness of TaskExecutorDistributor #45

jentfoo opened this issue Oct 22, 2013 · 3 comments

Comments

@jentfoo
Copy link
Member

jentfoo commented Oct 22, 2013

In the 0.8 release the TaskExecutorDistributor will get a form of fairness added to it, by limiting the number of tasks a key can execute before it releases control. This strategy works good when tasks are very uniform between each other.

It might make sense to have multiple TaskQueueWorker implementations such that you can have different fairness strategies (maybe constructed using the builder pattern). An example alternative strategy would be to limit the amount of time spent on a specific key (instead of execution count).

In order to implement a time strategy, or any strategy which would require checking while executing tasks would need to use a different TaskQueueWorker implementation so that if no fairness is desired, there is no performance loss to what we have today.

@jentfoo
Copy link
Member Author

jentfoo commented Nov 5, 2013

Because other fairness strategies would have performance impacts. And in addition I am not sure what use cases we would be trying to solve with additional strategies, I am going to wait on implementing this enhancement.

If anyone would like to see this, please post a comment about what fairness strategy, and why you would like to see it. That way I can be sure to implement something that will be useful to lots of people.

Right now the fairness strategy of limiting the amount of runnables per key at a time (possibly in combination with a sub pool), should solve most peoples use cases.

@jentfoo jentfoo added this to the 2.0.0 Release milestone Mar 17, 2014
@jentfoo
Copy link
Member Author

jentfoo commented Mar 17, 2014

I marked this for version 2.0.0 since to do this cleanly, we would have to make non-backwards compatible changes.

Depending on feedback, and how I feel about this as we get closer to 2.0.0 we may or may not want to include this additional functionality. But still marking for 2.0.0 milestone as that is the earliest we can do this (of course 2.0.0 can occur when ever want it to).

@jentfoo
Copy link
Member Author

jentfoo commented Apr 29, 2014

I am going to go ahead and close this as wont fix. This could still be done, I have a branch on from my local fork for 2.0.0, but worry this complicates things more than it is worth. As I stated above, if anyone is interested in something like this, let me know, we can re-open this. But I hate to add a builder pattern right now when the need is not clearly defined, and it adds complexity that is different from everything else in this project right now. If we have a need for more builders in the future, I will likely do this then as well.

@jentfoo jentfoo closed this as completed Apr 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant