Bounded Task Queue for ScheduledThreadPoolExecutor

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Bounded Task Queue for ScheduledThreadPoolExecutor

JSR166 Concurrency mailing list
Hello,

Given a ScheduledThreadPoolExecutor (STPE) with a set of slow consumers, the unbounded task queue constitutes the perfect disguise for backpressure-related problems. Producers just keep on pushing tasks to the point that 1) growing queue starts choking memory, 2) almost all pending time-sensitive tasks become obsolete when they are assigned to a thread to get executed, and 3) even relatively cheap tasks starve to death for no good reason. To the best of my knowledge, unfortunately, STPE does accept neither a bound, nor a custom queue in its ctor, even though the internally used ThreadPoolExecutor (TPE) perfectly allows that[1]. To work around the problem, I create my own ScheduledExecutorService where time-sensitive tasks are delegated to an internal STPE and the rest to a TPE. That being said, I would rather prefer STPE to just accept a custom implementation or a bound for the task queue it employs.

I plan to create a ticket for such a feature in OpenJDK. Before doing that, I would appreciate your feedback on my reasoning in case I am missing something obvious.

Best.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Bounded Task Queue for ScheduledThreadPoolExecutor

JSR166 Concurrency mailing list
On 4/23/19 2:37 AM, Volkan Yazıcı via Concurrency-interest wrote:

> Hello,
>
> Given a ScheduledThreadPoolExecutor (STPE) with a set of slow consumers,
> the unbounded task queue constitutes the perfect disguise for
> backpressure-related problems. Producers just keep on pushing tasks to
> the point that 1) growing queue starts choking memory, 2) almost all
> pending time-sensitive tasks become obsolete when they are assigned to a
> thread to get executed, and 3) even relatively cheap tasks starve to
> death for no good reason. To the best of my knowledge, unfortunately,
> STPE does accept neither a bound, nor a custom queue in its ctor, even
> though the internally used ThreadPoolExecutor (TPE) perfectly allows
> that[1]. To work around the problem, I create my own
> ScheduledExecutorService where time-sensitive tasks are delegated to an
> internal STPE and the rest to a TPE. That being said, I would rather
> prefer STPE to just accept a custom implementation or a bound for the
> task queue it employs.

STPE started out (long ago, pre-release) allowing custom queues,
defaulting to DelayQueue. This was changed to use an internal
specialization of DelayQueue to allow cleanup of cancelled tasks, which
helps significantly in networking applications in which timeout
callbacks rarely turn out to be needed.

At this point there is no reasonable path to reinstating a custom queue
option. However, it would be possible to add yet another configuration
method setMaximumQueueSize, which would cause a
RejectedExecutionException if exceeded.

We worry when adding such methods about unknown interactions with other
configuration methods and constructors, leading to future bug reports.
Which I think has happened every time we have added one.

In the mean time you could get most of this effect at reasonable cost by
wrapping STPE within another Executor that rejects when
stpe.getQueue().size() exceeds a threshold.

-Doug


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Bounded Task Queue for ScheduledThreadPoolExecutor

JSR166 Concurrency mailing list
Thanks so much for taking time to reply professor.

STPE.DelayWorkQueue ctor does not have any arguments and every access is synchronized on STPE.DelayWorkQueue#lock ReentrantLock, which I believe makes the enforcement of a bound "relatively easy". Would you mind sharing which sort of configurations you have in mind that might potentially lead to future bugs, please?

I see that you are more inclined to recommend me having my own custom STPE wrapper overriding ScheduledExecutorService methods with a check on getQueue().size() rather than creating a feature request in OpenJDK. Is my interpretation right?

Best.

On Wed, Apr 24, 2019 at 1:15 PM Doug Lea via Concurrency-interest <[hidden email]> wrote:
On 4/23/19 2:37 AM, Volkan Yazıcı via Concurrency-interest wrote:
> Hello,
>
> Given a ScheduledThreadPoolExecutor (STPE) with a set of slow consumers,
> the unbounded task queue constitutes the perfect disguise for
> backpressure-related problems. Producers just keep on pushing tasks to
> the point that 1) growing queue starts choking memory, 2) almost all
> pending time-sensitive tasks become obsolete when they are assigned to a
> thread to get executed, and 3) even relatively cheap tasks starve to
> death for no good reason. To the best of my knowledge, unfortunately,
> STPE does accept neither a bound, nor a custom queue in its ctor, even
> though the internally used ThreadPoolExecutor (TPE) perfectly allows
> that[1]. To work around the problem, I create my own
> ScheduledExecutorService where time-sensitive tasks are delegated to an
> internal STPE and the rest to a TPE. That being said, I would rather
> prefer STPE to just accept a custom implementation or a bound for the
> task queue it employs.

STPE started out (long ago, pre-release) allowing custom queues,
defaulting to DelayQueue. This was changed to use an internal
specialization of DelayQueue to allow cleanup of cancelled tasks, which
helps significantly in networking applications in which timeout
callbacks rarely turn out to be needed.

At this point there is no reasonable path to reinstating a custom queue
option. However, it would be possible to add yet another configuration
method setMaximumQueueSize, which would cause a
RejectedExecutionException if exceeded.

We worry when adding such methods about unknown interactions with other
configuration methods and constructors, leading to future bug reports.
Which I think has happened every time we have added one.

In the mean time you could get most of this effect at reasonable cost by
wrapping STPE within another Executor that rejects when
stpe.getQueue().size() exceeds a threshold.

-Doug


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Bounded Task Queue for ScheduledThreadPoolExecutor

JSR166 Concurrency mailing list
On Thu, 25 Apr 2019 at 09:14, Volkan Yazıcı via Concurrency-interest
<[hidden email]> wrote:
>
> Thanks so much for taking time to reply professor.
>
> STPE.DelayWorkQueue ctor does not have any arguments and every access is synchronized on STPE.DelayWorkQueue#lock ReentrantLock, which I believe makes the enforcement of a bound "relatively easy". Would you mind sharing which sort of configurations you have in mind that might potentially lead to future bugs, please?

STPE.DelayWorkQueue provides O(log N) removal time because each node
(future task) maintain its own index into the heap. This is not
possible if you provide a custom java.util.Queue/DelayQueue, because
it has no concept of index. It could be heap based or linked based or
something else. So the only generic way to remove a element is to
(potential) iterate over every item in the queue.

Also, see the original bug
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6602600

/Kasper
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Bounded Task Queue for ScheduledThreadPoolExecutor

JSR166 Concurrency mailing list
We are totally on the same page on the impact of allowing custom queues, that indeed does not look plausible unless there is a convincing need for it, which I doubt at this stage. In my previous e-mail, I was particularly referring to adding a setMaximumQueueSize() as Doug hinted. I do expect that to be relatively easy fix. What do you think?

On Thu, Apr 25, 2019 at 10:34 AM Kasper Nielsen <[hidden email]> wrote:
On Thu, 25 Apr 2019 at 09:14, Volkan Yazıcı via Concurrency-interest
<[hidden email]> wrote:
>
> Thanks so much for taking time to reply professor.
>
> STPE.DelayWorkQueue ctor does not have any arguments and every access is synchronized on STPE.DelayWorkQueue#lock ReentrantLock, which I believe makes the enforcement of a bound "relatively easy". Would you mind sharing which sort of configurations you have in mind that might potentially lead to future bugs, please?

STPE.DelayWorkQueue provides O(log N) removal time because each node
(future task) maintain its own index into the heap. This is not
possible if you provide a custom java.util.Queue/DelayQueue, because
it has no concept of index. It could be heap based or linked based or
something else. So the only generic way to remove a element is to
(potential) iterate over every item in the queue.

Also, see the original bug
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6602600

/Kasper

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Bounded Task Queue for ScheduledThreadPoolExecutor

JSR166 Concurrency mailing list
In reply to this post by JSR166 Concurrency mailing list
On 4/25/19 4:12 AM, Volkan Yazıcı via Concurrency-interest wrote:
> Thanks so much for taking time to reply professor.
>
> STPE.DelayWorkQueue ctor does not have any arguments and every access is
> synchronized on STPE.DelayWorkQueue#lock ReentrantLock, which I believe
> makes the enforcement of a bound "relatively easy". Would you mind
> sharing which sort of configurations you have in mind that might
> potentially lead to future bugs, please?

Possibilities include surprises about possible inconsistencies between
task rejection due to capacity vs shutdown status, since these need not
be atomically connected as users might expect them to be. If users are
required to do it themselves (as below), they can't be surprised by this.

>
> I see that you are more inclined to recommend me having my own custom
> STPE wrapper overriding ScheduledExecutorService methods with a check on
> getQueue().size() rather than creating a feature request in OpenJDK. Is
> my interpretation right?

Yes. To make this option more attractive, we might be able to relax some
of the internal locking, that would make this slightly faster.

-Doug

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest