From 9671115796e42865eaebd020ac27542802027b02 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Wed, 23 Dec 2020 12:37:54 -0800 Subject: [PATCH] Use 'workers' value from 'Config::figment()'. This commit also improves config pretty-printing and warning messages. It also fixes an issue that resulted in config value deprecation warnings not being emitted. The 'workers' value is now a 'usize', not a 'u16'; contrib pool sizes now default to 'workers * 2'. Closes #1470. --- contrib/lib/src/databases.rs | 8 +-- core/lib/Cargo.toml | 4 +- core/lib/src/config/config.rs | 101 ++++++++++++++++++++++-------- core/lib/src/config/mod.rs | 11 ++++ core/lib/src/config/secret_key.rs | 19 ++++++ core/lib/src/fairing/mod.rs | 2 +- core/lib/src/lib.rs | 5 +- core/lib/src/rocket.rs | 2 +- examples/config/Cargo.toml | 2 +- site/guide/9-configuration.md | 13 +++- site/tests/Cargo.toml | 2 +- 11 files changed, 131 insertions(+), 38 deletions(-) diff --git a/contrib/lib/src/databases.rs b/contrib/lib/src/databases.rs index 8aa6ce2d..7a8caf04 100644 --- a/contrib/lib/src/databases.rs +++ b/contrib/lib/src/databases.rs @@ -131,7 +131,7 @@ //! Additionally, all configurations accept the following _optional_ keys: //! //! * `pool_size` - the size of the pool, i.e., the number of connections to -//! pool (defaults to the configured number of workers) +//! pool (defaults to the configured number of workers * 2) //! //! Additional options may be required or supported by other adapters. //! @@ -424,7 +424,7 @@ use self::r2d2::ManageConnection; pub struct Config { /// Connection URL specified in the Rocket configuration. pub url: String, - /// Initial pool size. Defaults to the number of Rocket workers. + /// Initial pool size. Defaults to the number of Rocket workers * 2. pub pool_size: u32, /// How long to wait, in seconds, for a new connection before timing out. /// Defaults to `5`. @@ -462,7 +462,7 @@ impl Config { /// /// let config = Config::from("my_other_db", rocket).unwrap(); /// assert_eq!(config.url, "mysql://root:root@localhost/database"); - /// assert_eq!(config.pool_size, rocket.config().workers as u32); + /// assert_eq!(config.pool_size, (rocket.config().workers * 2) as u32); /// /// let config = Config::from("unknown_db", rocket); /// assert!(config.is_err()) @@ -477,7 +477,7 @@ impl Config { let db_key = format!("databases.{}", db_name); let key = |name: &str| format!("{}.{}", db_key, name); Figment::from(rocket.figment()) - .merge(Serialized::default(&key("pool_size"), rocket.config().workers)) + .merge(Serialized::default(&key("pool_size"), rocket.config().workers * 2)) .merge(Serialized::default(&key("timeout"), 5)) .extract_inner::(&db_key) } diff --git a/core/lib/Cargo.toml b/core/lib/Cargo.toml index b91df794..c99ebcec 100644 --- a/core/lib/Cargo.toml +++ b/core/lib/Cargo.toml @@ -41,7 +41,7 @@ atomic = "0.5" parking_lot = "0.11" ubyte = {version = "0.10", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } -figment = { version = "0.9.2", features = ["toml", "env"] } +figment = { version = "0.10", features = ["toml", "env"] } rand = "0.7" either = "1" @@ -55,7 +55,7 @@ version_check = "0.9.1" [dev-dependencies] bencher = "0.1" -figment = { version = "0.9.2", features = ["test"] } +figment = { version = "0.10", features = ["test"] } [[bench]] name = "format-routing" diff --git a/core/lib/src/config/config.rs b/core/lib/src/config/config.rs index 849b68d5..c896c875 100644 --- a/core/lib/src/config/config.rs +++ b/core/lib/src/config/config.rs @@ -25,7 +25,7 @@ use crate::data::Limits; /// the appropriate of the two based on the selected profile. With the exception /// of `log_level`, which is `normal` in `debug` and `critical` in `release`, /// and `secret_key`, which is regenerated from a random value if not set in -/// "debug" mode only, all of the values are identical in either profile. +/// "debug" mode only, all default values are identical in all profiles. /// /// # Provider Details /// @@ -57,8 +57,8 @@ pub struct Config { pub address: IpAddr, /// Port to serve on. **(default: `8000`)** pub port: u16, - /// Number of threads to use for executing futures. **(default: `cores * 2`)** - pub workers: u16, + /// Number of future-executing threads. **(default: `num cores`)** + pub workers: usize, /// Keep-alive timeout in seconds; disabled when `0`. **(default: `5`)** pub keep_alive: u32, /// Max level to log. **(default: _debug_ `normal` / _release_ `critical`)** @@ -130,7 +130,7 @@ impl Config { Config { address: Ipv4Addr::new(127, 0, 0, 1).into(), port: 8000, - workers: num_cpus::get() as u16 * 2, + workers: num_cpus::get(), keep_alive: 5, log_level: LogLevel::Normal, cli_colors: true, @@ -215,16 +215,7 @@ impl Config { /// let config = rocket::Config::from(figment); /// ``` pub fn from(provider: T) -> Self { - // Check for now depreacted config values. let figment = Figment::from(&provider); - for (key, replacement) in Self::DEPRECATED_KEYS { - if figment.find_value(key).is_ok() { - warn!("found value for deprecated config key `{}`", Paint::white(key)); - if let Some(new_key) = replacement { - info_!("key has been by replaced by `{}`", Paint::white(new_key)); - } - } - } #[allow(unused_mut)] let mut config = figment.extract::().unwrap_or_else(|e| { @@ -233,16 +224,12 @@ impl Config { }); #[cfg(all(feature = "secrets", not(test), not(rocket_unsafe_secret_key)))] { - if config.secret_key.is_zero() { + if !config.secret_key.is_provided() { if figment.profile() != Self::DEBUG_PROFILE { crate::logger::try_init(LogLevel::Debug, true, false); - error!("secrets enabled in `release` without `secret_key`"); + error!("secrets enabled in non-`debug` without `secret_key`"); info_!("disable `secrets` feature or configure a `secret_key`"); panic!("aborting due to configuration error(s)") - } else { - warn!("secrets enabled in `debug` without `secret_key`"); - info_!("disable `secrets` feature or configure a `secret_key`"); - info_!("this becomes a hard error in `release`"); } // in debug, generate a key for a bit more security @@ -272,10 +259,11 @@ impl Config { cfg!(feature = "tls") && self.tls.is_some() } - pub(crate) fn pretty_print(&self, profile: &Profile) { + pub(crate) fn pretty_print(&self, figment: &Figment) { use crate::logger::PaintExt; - launch_info!("{}Configured for {}.", Paint::emoji("🔧 "), profile); + launch_info!("{}Configured for {}.", Paint::emoji("🔧 "), figment.profile()); + launch_info_!("address: {}", Paint::default(&self.address).bold()); launch_info_!("port: {}", Paint::default(&self.port).bold()); launch_info_!("workers: {}", Paint::default(self.workers).bold()); @@ -295,6 +283,26 @@ impl Config { true => launch_info_!("tls: {}", Paint::default("enabled").bold()), false => launch_info_!("tls: {}", Paint::default("disabled").bold()), } + + if !self.secret_key.is_provided() { + warn!("secrets enabled without a configured `secret_key`"); + info_!("disable `secrets` feature or configure a `secret_key`"); + info_!("this becomes a {} in non-debug profiles", Paint::red("hard error").bold()); + } + + // Check for now depreacted config values. + for (key, replacement) in Self::DEPRECATED_KEYS { + if let Some(md) = figment.find_metadata(key) { + warn!("found value for deprecated config key `{}`", Paint::white(key)); + if let Some(ref source) = md.source { + info_!("in {} {}", Paint::white(source), md.name); + } + + if let Some(new_key) = replacement { + info_!("key has been by replaced by `{}`", Paint::white(new_key)); + } + } + } } } @@ -315,21 +323,62 @@ impl Provider for Config { #[doc(hidden)] pub fn pretty_print_error(error: figment::Error) { + use figment::error::{Kind, OneOf}; + crate::logger::try_init(LogLevel::Debug, true, false); for e in error { - error!("{}", e.kind); + fn w(v: T) -> Paint { Paint::white(v) } + + match e.kind { + Kind::Message(msg) => error_!("{}", msg), + Kind::InvalidType(v, exp) => { + error_!("invalid type: found {}, expected {}", w(v), w(exp)); + } + Kind::InvalidValue(v, exp) => { + error_!("invalid value {}, expected {}", w(v), w(exp)); + }, + Kind::InvalidLength(v, exp) => { + error_!("invalid length {}, expected {}", w(v), w(exp)) + }, + Kind::UnknownVariant(v, exp) => { + error_!("unknown variant: found `{}`, expected `{}`", w(v), w(OneOf(exp))) + } + Kind::UnknownField(v, exp) => { + error_!("unknown field: found `{}`, expected `{}`", w(v), w(OneOf(exp))) + } + Kind::MissingField(v) => { + error_!("missing field `{}`", w(v)) + } + Kind::DuplicateField(v) => { + error_!("duplicate field `{}`", w(v)) + } + Kind::ISizeOutOfRange(v) => { + error_!("signed integer `{}` is out of range", w(v)) + } + Kind::USizeOutOfRange(v) => { + error_!("unsigned integer `{}` is out of range", w(v)) + } + Kind::Unsupported(v) => { + error_!("unsupported type `{}`", w(v)) + } + Kind::UnsupportedKey(a, e) => { + error_!("unsupported type `{}` for key: must be `{}`", w(a), w(e)) + } + } if let (Some(ref profile), Some(ref md)) = (&e.profile, &e.metadata) { if !e.path.is_empty() { let key = md.interpolate(profile, &e.path); - info_!("for key {}", Paint::white(key)); + info_!("for key {}", w(key)); } } - if let Some(ref md) = e.metadata { - if let Some(ref source) = md.source { - info_!("in {} {}", Paint::white(source), md.name); + if let Some(md) = e.metadata { + if let Some(source) = md.source { + info_!("in {} {}", w(source), md.name); + } else { + info_!("in {}", w(md.name)); } } } diff --git a/core/lib/src/config/mod.rs b/core/lib/src/config/mod.rs index e134b05a..2339567c 100644 --- a/core/lib/src/config/mod.rs +++ b/core/lib/src/config/mod.rs @@ -30,6 +30,17 @@ //! [`Rocket::figment()`]: crate::Rocket::figment() //! [`Deserialize`]: serde::Deserialize //! +//! ## Workers +//! +//! The `workers` parameter sets the number of threads used for parallel task +//! execution; there is no limit to the number of concurrent tasks. Due to a +//! limitation in upstream async executers, unlike other values, the `workers` +//! configuration value cannot be reconfigured or be configured from sources +//! other than those provided by [`Config::figment()`]. In other words, only the +//! values set by the `ROCKET_WORKERS` environment variable or in the `workers` +//! property of `Rocket.toml` will be considered - all other `workers` values +//! are ignored. +//! //! ## Custom Providers //! //! A custom provider can be set via [`rocket::custom()`], which replaces calls to diff --git a/core/lib/src/config/secret_key.rs b/core/lib/src/config/secret_key.rs index 6688b6bf..e81256c7 100644 --- a/core/lib/src/config/secret_key.rs +++ b/core/lib/src/config/secret_key.rs @@ -125,6 +125,25 @@ impl SecretKey { pub fn is_zero(&self) -> bool { self.kind == Kind::Zero } + + /// Returns `true` if `self` was not automatically generated and is not zero. + /// + /// # Example + /// + /// ```rust + /// use rocket::config::SecretKey; + /// + /// let master = vec![0u8; 64]; + /// let key = SecretKey::generate().unwrap(); + /// assert!(!key.is_provided()); + /// + /// let master = vec![0u8; 64]; + /// let key = SecretKey::from(&master); + /// assert!(!key.is_provided()); + /// ``` + pub fn is_provided(&self) -> bool { + self.kind == Kind::Provided + } } #[doc(hidden)] diff --git a/core/lib/src/fairing/mod.rs b/core/lib/src/fairing/mod.rs index b7d14620..76482d5a 100644 --- a/core/lib/src/fairing/mod.rs +++ b/core/lib/src/fairing/mod.rs @@ -113,7 +113,7 @@ pub use self::info_kind::{Info, Kind}; /// An attach callback can arbitrarily modify the `Rocket` instance being /// constructed. It returns `Ok` if it would like launching to proceed /// nominally and `Err` otherwise. If an attach callback returns `Err`, -/// launch will be aborted. All attach callbacks are executed on `launch`, +/// launch will be aborted. All attach callbacks are executed on `attach`, /// even if one or more signal a failure. /// /// * **Launch (`on_launch`)** diff --git a/core/lib/src/lib.rs b/core/lib/src/lib.rs index 6992b5e4..6d98739e 100644 --- a/core/lib/src/lib.rs +++ b/core/lib/src/lib.rs @@ -161,7 +161,6 @@ pub fn custom(provider: T) -> Rocket { Rocket::custom(provider) } -// TODO.async: More thoughtful plan for async tests /// WARNING: This is unstable! Do not use this method outside of Rocket! #[doc(hidden)] pub fn async_test(fut: impl std::future::Future + Send) -> R { @@ -178,8 +177,12 @@ pub fn async_test(fut: impl std::future::Future + Send) -> R { /// WARNING: This is unstable! Do not use this method outside of Rocket! #[doc(hidden)] pub fn async_main(fut: impl std::future::Future + Send) -> R { + // FIXME: The `workers` value won't reflect swaps of `Rocket` in attach + // fairings with different config values, or values from non-Rocket configs. + // See tokio-rs/tokio#3329 for a necessary solution in `tokio`. tokio::runtime::Builder::new() .threaded_scheduler() + .core_threads(Config::from(Config::figment()).workers) .thread_name("rocket-worker-thread") .enable_all() .build() diff --git a/core/lib/src/rocket.rs b/core/lib/src/rocket.rs index ad401483..03752759 100644 --- a/core/lib/src/rocket.rs +++ b/core/lib/src/rocket.rs @@ -83,7 +83,7 @@ impl Rocket { pub fn custom(provider: T) -> Rocket { let (config, figment) = (Config::from(&provider), Figment::from(provider)); logger::try_init(config.log_level, config.cli_colors, false); - config.pretty_print(figment.profile()); + config.pretty_print(&figment); let managed_state = Container::new(); let (shutdown_sender, shutdown_receiver) = mpsc::channel(1); diff --git a/examples/config/Cargo.toml b/examples/config/Cargo.toml index 553a2f53..6489adda 100644 --- a/examples/config/Cargo.toml +++ b/examples/config/Cargo.toml @@ -6,4 +6,4 @@ edition = "2018" publish = false [dependencies] -rocket = { path = "../../core/lib" } +rocket = { path = "../../core/lib", features = ["secrets"] } diff --git a/site/guide/9-configuration.md b/site/guide/9-configuration.md index 22ad22f4..5aeae898 100644 --- a/site/guide/9-configuration.md +++ b/site/guide/9-configuration.md @@ -21,7 +21,7 @@ values: |----------------|-----------------|-------------------------------------------------|-----------------------| | `address` | `IpAddr` | IP address to serve on | `127.0.0.1` | | `port` | `u16` | Port to serve on. | `8000` | -| `workers` | `u16` | Number of threads to use for executing futures. | cpu core count * 2 | +| `workers` | `usize` | Number of threads to use for executing futures. | cpu core count | | `keep_alive` | `u32` | Keep-alive timeout seconds; disabled when `0`. | `5` | | `log_level` | `LogLevel` | Max level to log. (off/normal/debug/critical) | `normal`/`critical` | | `cli_colors` | `bool` | Whether to use colors and emoji when logging. | `true` | @@ -110,6 +110,17 @@ file's directory. ! warning: Rocket's built-in TLS implements only TLS 1.2 and 1.3. As such, it may not be suitable for production use. +### Workers + +The `workers` parameter sets the number of threads used for parallel task +execution; there is no limit to the number of concurrent tasks. Due to a +limitation in upstream async executers, unlike other values, the `workers` +configuration value cannot be reconfigured or be configured from sources other +than those provided by [`Config::figment()`], detailed below. In other words, +only the values set by the `ROCKET_WORKERS` environment variable or in the +`workers` property of `Rocket.toml` will be considered - all other `workers` +values are ignored. + ## Default Provider Rocket's default configuration provider is [`Config::figment()`]; this is the diff --git a/site/tests/Cargo.toml b/site/tests/Cargo.toml index 5cf121a9..bf92e5ca 100644 --- a/site/tests/Cargo.toml +++ b/site/tests/Cargo.toml @@ -11,4 +11,4 @@ doc-comment = "0.3" rocket_contrib = { path = "../../contrib/lib", features = ["json", "tera_templates", "diesel_sqlite_pool"] } serde = { version = "1.0", features = ["derive"] } rand = "0.7" -figment = { version = "0.9.2", features = ["toml", "env"] } +figment = { version = "0.10", features = ["toml", "env"] }