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

Async HTTP thread pool issues #405

Closed
mattheworiordan opened this Issue Jul 9, 2018 · 4 comments

Comments

2 participants
@mattheworiordan
Copy link
Member

mattheworiordan commented Jul 9, 2018

The threadpool for the asyncScheduler has a maximum of 64 threads and it's a hard-coded number in the library.

Currently it doesn't look like there's a public way to modify that.
It would be useful to be able to configure that when the library is instanced

@paddybyers

This comment has been minimized.

Copy link
Member

paddybyers commented Jul 11, 2018

There's no public API - we should add an API to configure the min/max threads and possibly also the cooldown period for idle threads.

However, there's a bigger issue which is that the threadpool isn't actually growing on demand as we expected. A customer has reported:

This is caused by this implementation:

public AsyncHttpScheduler(HttpCore httpCore) {
        super(httpCore, new ThreadPoolExecutor(DEFAULT_POOL_SIZE, MAX_POOL_SIZE, KEEP_ALIVE_TIME, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>()));
    }

According to the ThreadPoolExecutor.execute() documentation, a new thread is only spawned when the pool queue is full. As linked blocking queue is unbounded, no new thread is ever spawned and all requests are done within one thread.

@paddybyers paddybyers changed the title Configurable HTTP thread pool count Async HTTP thread pool issues Jul 11, 2018

@paddybyers

This comment has been minimized.

Copy link
Member

paddybyers commented Jul 11, 2018

The behaviour I propose is as follows:

  • there is a configured maximum number of threads, which can be overridden via a client option;
  • the threadpool is initially empty;
  • as requests are made, threads are created, without waiting, to service the request;
  • when the max number of threads is reached, further requests are queued;
  • there is no limit to the number of requests that can be queued;
  • any threads created will exit if they are idle for a period of time.

My reading of the spec is that we can do this - ie create threads as a first preference, and queue as a fallback - by setting the core pool size to the maximum, and allowing idle threads to exit as follows:

  • add maxAsyncHttpThreads to ClientOptions with a default of, say, 64.
  • construct the ThreadPoolExecutor as follows:
	public AsyncHttpScheduler(HttpCore httpCore, ClientOptions options) {
		super(httpCore, new ThreadPoolExecutor(options.maxAsyncHttpThreads, options.maxAsyncHttpThreads, KEEP_ALIVE_TIME, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>()));
		executor.allowCoreThreadTimeOut(true);
	}
@mattheworiordan

This comment has been minimized.

Copy link
Member

mattheworiordan commented Jul 11, 2018

@paddybyers sounds good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment