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

Deprecate and replace Runtime::threadpool_builder #645

Merged
merged 2 commits into from
Sep 19, 2018

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Sep 18, 2018

Problem

Currently, the concurrent Runtime is configured by passing in a
threadpool builder. This is problematic for two reasons:

  1. It exposes tokio-threadpool as part of the tokio public API,
    preventing tokio from transparently bumping the version used
    internally.
  2. All the options on the threadpool builder cannot be
    maintained. There are some that the Runtime must set, overwriting
    previously set values.

Solution

This branch makes the following changes:

  1. Hide and deprecate threadpool_builder.
  2. Add core_threads representing pool_size on the threadpool builder.
  3. Add blocking_threads representing max_blocking on the threadpool builder.
  4. Add name_prefix, mirroring the same fn on the threadpool builder.
  5. Add stack_size, mirroring the same fn on the threadpool builder.

Fixes: #642

Signed-off-by: Eliza Weisman eliza@buoyant.io

@hawkw hawkw added the C-enhancement Category: A PR with an enhancement or bugfix. label Sep 18, 2018
@hawkw hawkw self-assigned this Sep 18, 2018
Copy link
Member

@tobz tobz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Looks like the AppVeyor failure was due to a transient tokio-tls test failure?

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 :shipit:

@carllerche
Copy link
Member

I restarted CI and filed #647 to track fixing the spurious failure.

@tobz tobz merged commit 0ca973a into master Sep 19, 2018
@carllerche carllerche deleted the eliza/remove-threadpool-builder branch September 20, 2018 16:42
dekellum added a commit to dekellum/body-image that referenced this pull request Oct 14, 2018
tokio-rs/tokio#645 deprecated `tokio::runtime::Builder::threadpool_builder`
with the following message:

   use of deprecated item 'tokio::runtime::Builder::threadpool_builder':
   use the `core_threads`, `blocking_threads`, `name_prefix`, and
   `stack_size` functions on `runtime::Builder`, instead

The was originally released in tokio 0.1.9. Subsequently tokio 0.1.10
and 0.1.11 were released as (purely semver) bug fixes of
the former, so make 0.1.11 the new minimum along with this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A PR with an enhancement or bugfix.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime builder should not accept a tokio-threadpool builder.
3 participants