Skip to content

Commit 756a09e

Browse files
hushyclaude
andcommitted
fix(config): apply --service override before disk-usage validation
A searcher node started with `--service searcher` could log a misleading "data dir volume too small" warning at startup, comparing its volume against the indexer split-store and ingest queue disk budgets. A searcher does not use those budgets, so the warning was wrong and misleading. The cause was validation order: the CLI `--service` override was applied after `load_node_config()` had already run validation, so the disk-usage check ran against the default (all-services) set and counted indexer/ingest budgets. This threads the service override through `load_node_config_with_env` into `build_and_validate`, applying it where `enabled_services` is resolved, before validation. The override takes precedence over the config file and env vars, matching the previous post-load behavior. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a5ad540 commit 756a09e

5 files changed

Lines changed: 127 additions & 37 deletions

File tree

quickwit/quickwit-cli/src/lib.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,14 +230,25 @@ pub fn start_actor_runtimes(
230230
}
231231

232232
/// Loads a node config located at `config_uri` with the default storage configuration.
233-
async fn load_node_config(config_uri: &Uri) -> anyhow::Result<NodeConfig> {
233+
///
234+
/// When `service_override` is provided, it is applied before config validation so that
235+
/// service-dependent validation (e.g. the disk-usage check) reflects the actual runtime service
236+
/// set (e.g. `--service searcher`).
237+
async fn load_node_config(
238+
config_uri: &Uri,
239+
service_override: Option<&HashSet<QuickwitService>>,
240+
) -> anyhow::Result<NodeConfig> {
234241
let config_content = load_file(&StorageResolver::unconfigured(), config_uri)
235242
.await
236243
.context("failed to load node config")?;
237244
let config_format = ConfigFormat::sniff_from_uri(config_uri)?;
238-
let config = NodeConfig::load(config_format, config_content.as_slice())
239-
.await
240-
.with_context(|| format!("failed to parse node config `{config_uri}`"))?;
245+
let config = NodeConfig::load_with_service_override(
246+
config_format,
247+
config_content.as_slice(),
248+
service_override,
249+
)
250+
.await
251+
.with_context(|| format!("failed to parse node config `{config_uri}`"))?;
241252
info!(config_uri=%config_uri, config=?config, "loaded node config");
242253
Ok(config)
243254
}

quickwit/quickwit-cli/src/service.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,13 @@ impl RunCliCommand {
117117
debug!(args = ?self, "run-service");
118118
let version_text = BuildInfo::get_version_text();
119119
info!("quickwit version: {version_text}");
120-
let mut node_config = load_node_config(&self.config_uri).await?;
120+
let node_config = load_node_config(&self.config_uri, self.services.as_ref()).await?;
121+
if let Some(services) = &self.services {
122+
info!(services = %services.iter().join(", "), "services set from CLI override");
123+
}
121124
let (storage_resolver, metastore_resolver) =
122125
get_resolvers(&node_config.storage_configs, &node_config.metastore_configs);
123126
crate::busy_detector::set_enabled(true);
124-
125-
if let Some(services) = &self.services {
126-
info!(services = %services.iter().join(", "), "setting services from override");
127-
node_config.enabled_services.clone_from(services);
128-
}
129127
// TODO move in serve quickwit?
130128
let runtimes_config = RuntimesConfig::default();
131129
start_actor_runtimes(runtimes_config, &node_config.enabled_services)?;

quickwit/quickwit-cli/src/tool.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ pub async fn local_ingest_docs_cli(args: LocalIngestDocsArgs) -> anyhow::Result<
402402
debug!(args=?args, "local-ingest-docs");
403403
println!("❯ Ingesting documents locally...");
404404

405-
let config = load_node_config(&args.config_uri).await?;
405+
let config = load_node_config(&args.config_uri, None).await?;
406406
let (storage_resolver, metastore_resolver) =
407407
get_resolvers(&config.storage_configs, &config.metastore_configs);
408408
let mut metastore = metastore_resolver.resolve(&config.metastore_uri).await?;
@@ -536,7 +536,7 @@ pub async fn local_ingest_docs_cli(args: LocalIngestDocsArgs) -> anyhow::Result<
536536
pub async fn local_search_cli(args: LocalSearchArgs) -> anyhow::Result<()> {
537537
debug!(args=?args, "local-search");
538538
println!("❯ Searching directly on the index storage (without calling REST API)...");
539-
let config = load_node_config(&args.config_uri).await?;
539+
let config = load_node_config(&args.config_uri, None).await?;
540540
let (storage_resolver, metastore_resolver) =
541541
get_resolvers(&config.storage_configs, &config.metastore_configs);
542542
let metastore: MetastoreServiceClient =
@@ -574,7 +574,7 @@ pub async fn local_search_cli(args: LocalSearchArgs) -> anyhow::Result<()> {
574574
pub async fn merge_cli(args: MergeArgs) -> anyhow::Result<()> {
575575
debug!(args=?args, "run-merge-operations");
576576
println!("❯ Merging splits locally...");
577-
let config = load_node_config(&args.config_uri).await?;
577+
let config = load_node_config(&args.config_uri, None).await?;
578578
let (storage_resolver, metastore_resolver) =
579579
get_resolvers(&config.storage_configs, &config.metastore_configs);
580580
let mut metastore = metastore_resolver.resolve(&config.metastore_uri).await?;
@@ -662,7 +662,7 @@ pub async fn garbage_collect_index_cli(args: GarbageCollectIndexArgs) -> anyhow:
662662
debug!(args=?args, "garbage-collect-index");
663663
println!("❯ Garbage collecting index...");
664664

665-
let config = load_node_config(&args.config_uri).await?;
665+
let config = load_node_config(&args.config_uri, None).await?;
666666
let (storage_resolver, metastore_resolver) =
667667
get_resolvers(&config.storage_configs, &config.metastore_configs);
668668
let metastore = metastore_resolver.resolve(&config.metastore_uri).await?;
@@ -792,7 +792,7 @@ async fn extract_split_cli(args: ExtractSplitArgs) -> anyhow::Result<()> {
792792
debug!(args=?args, "extract-split");
793793
println!("❯ Extracting split...");
794794

795-
let config = load_node_config(&args.config_uri).await?;
795+
let config = load_node_config(&args.config_uri, None).await?;
796796
let (storage_resolver, metastore_resolver) =
797797
get_resolvers(&config.storage_configs, &config.metastore_configs);
798798
let metastore = metastore_resolver.resolve(&config.metastore_uri).await?;

quickwit/quickwit-config/src/node_config/mod.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,8 +911,22 @@ impl NodeConfig {
911911

912912
/// Parses and validates a [`NodeConfig`] from a given URI and config content.
913913
pub async fn load(config_format: ConfigFormat, config_content: &[u8]) -> anyhow::Result<Self> {
914+
Self::load_with_service_override(config_format, config_content, None).await
915+
}
916+
917+
/// Parses and validates a [`NodeConfig`], applying `service_override` before validation so
918+
/// service-dependent validation (e.g. the disk-usage check) uses the actual runtime service
919+
/// set rather than the pre-override default (e.g. when `--service searcher` is passed via the
920+
/// CLI).
921+
pub async fn load_with_service_override(
922+
config_format: ConfigFormat,
923+
config_content: &[u8],
924+
service_override: Option<&HashSet<QuickwitService>>,
925+
) -> anyhow::Result<Self> {
914926
let env_vars = env::vars().collect::<HashMap<_, _>>();
915-
let config = load_node_config_with_env(config_format, config_content, &env_vars).await?;
927+
let config =
928+
load_node_config_with_env(config_format, config_content, service_override, &env_vars)
929+
.await?;
916930
if !config.data_dir_path.try_exists()? {
917931
bail!(
918932
"data dir `{}` does not exist",

0 commit comments

Comments
 (0)