diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 4e69692a654..d23505abc58 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -26,6 +26,8 @@ Published Pants binaries are now compiled with a new `dist` Cargo profile that e Pants option config files are now parsed as TOML 1.1 rather than TOML 1.0. This covers `pants.toml` and any other file named by `[GLOBAL].pants_config_files`, the rcfiles named by `[GLOBAL].pantsrc_files` (`/etc/pantsrc`, `~/.pants.rc` and `.pants.rc` by default), and `.toml` files referenced by `@fromfile` option values. Inline tables may now span multiple lines and end with a trailing comma, strings may use the `\e` and `\xHH` escapes, and times may omit their seconds. TOML 1.1 only adds syntax to TOML 1.0, so existing files continue to parse unchanged. TOML files read by backends, such as `pyproject.toml`, are unaffected. +The `remote_cache_rpc_timeout_millis` & `remote_cache_rpc_concurrency` options are now correctly used again after being silently ignored since 2.19. The documented default for `remote_cache_rpc_timeout_millis` is now `30000` to align with the actual behavior since 2.19. If you used the defaults then nothing changes; if you explicitly set them they now take effect. + ### Goals ### Backends diff --git a/src/python/pants/option/bootstrap_options.py b/src/python/pants/option/bootstrap_options.py index 75894f90ec2..6484c27045f 100644 --- a/src/python/pants/option/bootstrap_options.py +++ b/src/python/pants/option/bootstrap_options.py @@ -764,7 +764,9 @@ def from_options(cls, options: OptionValueContainer) -> LocalStoreOptions: # Remote cache setup. remote_cache_warnings=RemoteCacheWarningsBehavior.backoff, remote_cache_rpc_concurrency=128, - remote_cache_rpc_timeout_millis=1500, + # NB: Matches the store RPC timeout: from 2.19 until this option's wiring was restored, cache + # RPCs accidentally used the store timeout, so this preserves the effective default. + remote_cache_rpc_timeout_millis=30000, # Remote execution setup. remote_execution_address=None, remote_execution_headers={}, diff --git a/src/rust/engine/src/context.rs b/src/rust/engine/src/context.rs index f3f28ac451a..9d0184d202f 100644 --- a/src/rust/engine/src/context.rs +++ b/src/rust/engine/src/context.rs @@ -144,6 +144,19 @@ impl RemotingOptions { batch_load_enabled: self.store_batch_load_enabled, }) } + + /// Options for the remote ActionCache provider: identical to the store options, except that + /// the cache-specific RPC timeout and concurrency limit apply instead of the store ones. + fn to_remote_cache_options( + &self, + tls_config: grpc_util::tls::Config, + ) -> Result { + Ok(RemoteStoreOptions { + timeout: self.cache_rpc_timeout, + concurrency_limit: self.cache_rpc_concurrency, + ..self.to_remote_store_options(tls_config)? + }) + } } #[derive(Clone, Debug)] @@ -393,7 +406,7 @@ impl Core { .append_only_caches_base_path .clone(), }, - remoting_opts.to_remote_store_options(tls_config)?, + remoting_opts.to_remote_cache_options(tls_config)?, ) .await?, ); @@ -1010,3 +1023,57 @@ impl SessionCore { self.backtrack_levels.lock().get(node).cloned().unwrap_or(0) } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + use std::time::Duration; + + use process_execution::CacheContentBehavior; + use remote::remote_cache::RemoteCacheWarningsBehavior; + use store::RemoteProvider; + + use super::RemotingOptions; + + #[test] + fn remote_cache_options_use_cache_rpc_settings() { + let remoting_opts = RemotingOptions { + provider: RemoteProvider::Reapi, + execution_enable: false, + store_address: Some("http://localhost:0".to_owned()), + execution_address: None, + execution_process_cache_namespace: None, + instance_name: None, + root_ca_certs_path: None, + client_certs_path: None, + client_key_path: None, + store_headers: BTreeMap::new(), + store_chunk_bytes: 1024, + store_rpc_retries: 2, + store_rpc_concurrency: 64, + store_rpc_timeout: Duration::from_secs(30), + store_batch_api_size_limit: 1024, + store_batch_load_enabled: false, + cache_warnings_behavior: RemoteCacheWarningsBehavior::FirstOnly, + cache_content_behavior: CacheContentBehavior::Fetch, + cache_rpc_concurrency: 128, + cache_rpc_timeout: Duration::from_secs(5), + execution_headers: BTreeMap::new(), + execution_overall_deadline: Duration::from_secs(60), + execution_rpc_concurrency: 1, + append_only_caches_base_path: None, + }; + + let tls_config = grpc_util::tls::Config::default(); + + let store_options = remoting_opts + .to_remote_store_options(tls_config.clone()) + .unwrap(); + assert_eq!(store_options.timeout, Duration::from_secs(30)); + assert_eq!(store_options.concurrency_limit, 64); + + let cache_options = remoting_opts.to_remote_cache_options(tls_config).unwrap(); + assert_eq!(cache_options.timeout, Duration::from_secs(5)); + assert_eq!(cache_options.concurrency_limit, 128); + } +}