Skip to content

Refactor TaskQueue to use RetryableQueue interface #861

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

PauloMigAlmeida
Copy link
Contributor

@PauloMigAlmeida PauloMigAlmeida commented May 26, 2025

Problem
When creating a custom <Executor className="myCoolClass">, developers may also want to use their own BlockingQueue<Runnable> implementation instead of the default TaskQueue.

While ThreadPoolExecutor accepts any BlockingQueue<Runnable> in its constructor, it assumes/expects the queue is a TaskQueue when handling RejectedExecutionException, as shown in the snippet below:

if (getQueue() instanceof TaskQueue queue) {
// If the Executor is close to maximum pool size, concurrent
// calls to execute() may result (due to Tomcat's use of
// TaskQueue) in some tasks being rejected rather than queued.
// If this happens, add them to the queue.
if (!queue.force(command)) {
submittedCount.decrementAndGet();
throw new RejectedExecutionException(sm.getString("threadPoolExecutor.queueFull"));
}

Proposed changes
Introduce a RetryableQueue<T> interface with the .force(T) method. This allows users to implement custom TaskQueue-like queues that can integrate with the existing rejection handling logic by implementing this interface.

@markt-asf
Copy link
Contributor

markt-asf commented May 27, 2025

This looks to be a reasonable request. There are a few additional changes that will also be required. I'll merge this, make the additional changes and then back-port the combination.

@markt-asf markt-asf merged commit 9b27e48 into apache:main May 27, 2025
markt-asf added a commit that referenced this pull request May 27, 2025
@PauloMigAlmeida
Copy link
Contributor Author

Thanks @markt-asf !

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.

2 participants