Metastore and index uris default to data-dir, Closes #1137 (#1140)

* [WIP] Refactor default uris in config, see #1137

* [WIP] Improve config getters and deploy to cli

* Test default index path config in data-dir

* Remove unused config renames

* Formatting

* Clippy fixes

* Impl config's Debug with resolved default uris

Co-authored-by: François Massot <[email protected]>

Author: ddelemeny

quickwit-cli/src/index.rs

@@ -471,7 +471,7 @@ pub async fn describe_index_cli(args: DescribeIndexArgs) -> anyhow::Result<()> {
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let quickwit_config = load_quickwit_config(args.config_uri, args.data_dir).await?;
     let metastore = metastore_uri_resolver
-        .resolve(&quickwit_config.metastore_uri)
+        .resolve(&quickwit_config.metastore_uri())
         .await?;
     let index_metadata = metastore.index_metadata(&args.index_id).await?;
     let splits = metastore
@@ -700,7 +700,8 @@ pub async fn create_index_cli(args: CreateIndexArgs) -> anyhow::Result<()> {
     } else {
         let default_index_uri = format!(
             "{}/{}",
-            quickwit_config.default_index_root_uri, index_config.index_id
+            quickwit_config.default_index_root_uri(),
+            index_config.index_id
         );
         info!(
             "`index-uri` is not specified in the index configuration. Setting it to `{}`.",
@@ -711,7 +712,7 @@ pub async fn create_index_cli(args: CreateIndexArgs) -> anyhow::Result<()> {
 
     if args.overwrite {
         delete_index(
-            &quickwit_config.metastore_uri,
+            &quickwit_config.metastore_uri(),
             &index_config.index_id,
             false,
         )
@@ -734,7 +735,7 @@ pub async fn create_index_cli(args: CreateIndexArgs) -> anyhow::Result<()> {
         create_timestamp: Utc::now().timestamp(),
         update_timestamp: Utc::now().timestamp(),
     };
-    create_index(&quickwit_config.metastore_uri, index_metadata.clone()).await?;
+    create_index(&quickwit_config.metastore_uri(), index_metadata.clone()).await?;
     println!("Index `{}` successfully created.", index_config.index_id);
 
     Ok(())
@@ -755,10 +756,10 @@ pub async fn ingest_docs_cli(args: IngestDocsArgs) -> anyhow::Result<()> {
         source_id: INGEST_SOURCE_ID.to_string(),
         source_params,
     };
-    run_index_checklist(&config.metastore_uri, &args.index_id, Some(&source)).await?;
+    run_index_checklist(&config.metastore_uri(), &args.index_id, Some(&source)).await?;
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&config.metastore_uri)
+        .resolve(&config.metastore_uri())
         .await?;
     let index_metadata = metastore.index_metadata(&args.index_id).await?;
     let storage_resolver = quickwit_storage_uri_resolver().clone();
@@ -808,7 +809,7 @@ pub async fn search_index(args: SearchIndexArgs) -> anyhow::Result<SearchRespons
     let storage_uri_resolver = quickwit_storage_uri_resolver();
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&quickwit_config.metastore_uri)
+        .resolve(&quickwit_config.metastore_uri())
         .await?;
     let search_request = SearchRequest {
         index_id: args.index_id,
@@ -841,13 +842,13 @@ pub async fn merge_or_demux_cli(
 ) -> anyhow::Result<()> {
     debug!(args = ?args, merge_enabled = merge_enabled, demux_enabled = demux_enabled, "run-merge-operations");
     let config = load_quickwit_config(args.config_uri, args.data_dir).await?;
-    run_index_checklist(&config.metastore_uri, &args.index_id, None).await?;
+    run_index_checklist(&config.metastore_uri(), &args.index_id, None).await?;
     let indexer_config = IndexerConfig {
         ..Default::default()
     };
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&config.metastore_uri)
+        .resolve(&config.metastore_uri())
         .await?;
     let storage_resolver = quickwit_storage_uri_resolver().clone();
     let client = IndexingServer::spawn(
@@ -872,8 +873,12 @@ pub async fn delete_index_cli(args: DeleteIndexArgs) -> anyhow::Result<()> {
     quickwit_telemetry::send_telemetry_event(TelemetryEvent::Delete).await;
 
     let quickwit_config = load_quickwit_config(args.config_uri, args.data_dir).await?;
-    let affected_files =
-        delete_index(&quickwit_config.metastore_uri, &args.index_id, args.dry_run).await?;
+    let affected_files = delete_index(
+        &quickwit_config.metastore_uri(),
+        &args.index_id,
+        args.dry_run,
+    )
+    .await?;
     if args.dry_run {
         if affected_files.is_empty() {
             println!("Only the index will be deleted since it does not contains any data file.");
@@ -898,7 +903,7 @@ pub async fn garbage_collect_index_cli(args: GarbageCollectIndexArgs) -> anyhow:
 
     let quickwit_config = load_quickwit_config(args.config_uri, args.data_dir).await?;
     let deleted_files = garbage_collect_index(
-        &quickwit_config.metastore_uri,
+        &quickwit_config.metastore_uri(),
         &args.index_id,
         args.grace_period,
         args.dry_run,

quickwit-cli/src/service.rs

@@ -153,7 +153,7 @@ async fn run_indexer_cli(args: RunIndexerArgs) -> anyhow::Result<()> {
 
     let config = load_quickwit_config(args.config_uri, args.data_dir_path).await?;
     let metastore = quickwit_metastore_uri_resolver()
-        .resolve(&config.metastore_uri)
+        .resolve(&config.metastore_uri())
         .await?;
     let storage_resolver = quickwit_storage_uri_resolver().clone();
     let client = IndexingServer::spawn(
@@ -180,7 +180,7 @@ async fn run_searcher_cli(args: RunSearcherArgs) -> anyhow::Result<()> {
     let config = load_quickwit_config(args.config_uri, args.data_dir_path).await?;
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&config.metastore_uri)
+        .resolve(&config.metastore_uri())
         .await?;
     run_checklist(vec![("metastore", metastore.check_connectivity().await)]);
     run_searcher(config, metastore).await?;

quickwit-cli/src/source.rs

@@ -224,7 +224,7 @@ impl SourceCliCommand {
 async fn add_source_cli(args: AddSourceArgs) -> anyhow::Result<()> {
     let config = load_quickwit_config(args.config_uri, None).await?;
     let metastore = quickwit_metastore_uri_resolver()
-        .resolve(&config.metastore_uri)
+        .resolve(&config.metastore_uri())
         .await?;
     let params = sniff_params(&args.params).await?;
     let mut source_params_json: Map<String, Value> = Map::new();
@@ -249,7 +249,7 @@ async fn add_source_cli(args: AddSourceArgs) -> anyhow::Result<()> {
 async fn delete_source_cli(args: DeleteSourceArgs) -> anyhow::Result<()> {
     let config = load_quickwit_config(args.config_uri, None).await?;
     let metastore = quickwit_metastore_uri_resolver()
-        .resolve(&config.metastore_uri)
+        .resolve(&config.metastore_uri())
         .await?;
     metastore
         .delete_source(&args.index_id, &args.source_id)
@@ -263,7 +263,7 @@ async fn delete_source_cli(args: DeleteSourceArgs) -> anyhow::Result<()> {
 
 async fn describe_source_cli(args: DescribeSourceArgs) -> anyhow::Result<()> {
     let quickwit_config = load_quickwit_config(args.config_uri, None).await?;
-    let index_metadata = resolve_index(&quickwit_config.metastore_uri, &args.index_id).await?;
+    let index_metadata = resolve_index(&quickwit_config.metastore_uri(), &args.index_id).await?;
     let source_checkpoint = index_metadata
         .checkpoint
         .source_checkpoint(&args.source_id)
@@ -316,7 +316,7 @@ where
 
 async fn list_sources_cli(args: ListSourcesArgs) -> anyhow::Result<()> {
     let quickwit_config = load_quickwit_config(args.config_uri, None).await?;
-    let index_metadata = resolve_index(&quickwit_config.metastore_uri, &args.index_id).await?;
+    let index_metadata = resolve_index(&quickwit_config.metastore_uri(), &args.index_id).await?;
     let table = make_list_sources_table(index_metadata.sources.into_values());
     display_tables(&[table]);
     Ok(())

quickwit-cli/src/split.rs

@@ -270,7 +270,7 @@ async fn list_split_cli(args: ListSplitArgs) -> anyhow::Result<()> {
     let quickwit_config = load_quickwit_config(args.config_uri, args.data_dir).await?;
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&quickwit_config.metastore_uri)
+        .resolve(&quickwit_config.metastore_uri())
         .await?;
     let splits = metastore.list_all_splits(&args.index_id).await?;
 
@@ -295,7 +295,7 @@ async fn describe_split_cli(args: DescribeSplitArgs) -> anyhow::Result<()> {
     let storage_uri_resolver = quickwit_storage_uri_resolver();
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&quickwit_config.metastore_uri)
+        .resolve(&quickwit_config.metastore_uri())
         .await?;
     let index_metadata = metastore.index_metadata(&args.index_id).await?;
     let index_storage = storage_uri_resolver.resolve(&index_metadata.index_uri)?;
@@ -325,7 +325,7 @@ async fn extract_split_cli(args: ExtractSplitArgs) -> anyhow::Result<()> {
     let storage_uri_resolver = quickwit_storage_uri_resolver();
     let metastore_uri_resolver = quickwit_metastore_uri_resolver();
     let metastore = metastore_uri_resolver
-        .resolve(&quickwit_config.metastore_uri)
+        .resolve(&quickwit_config.metastore_uri())
         .await?;
     let index_metadata = metastore.index_metadata(&args.index_id).await?;
     let index_storage = storage_uri_resolver.resolve(&index_metadata.index_uri)?;

quickwit-cli/tests/cli.rs

@@ -109,12 +109,7 @@ async fn test_cmd_create() -> Result<()> {
         .await
         .unwrap();
     assert_eq!(index_metadata.index_id, test_env.index_id);
-    assert_eq!(
-        index_metadata.index_uri,
-        Uri::try_new(&format!("./qwdata/indexes/{}", test_env.index_id))
-            .unwrap()
-            .as_ref()
-    );
+    assert_eq!(index_metadata.index_uri, test_env.index_uri.as_ref());
 
     // Attempt to create with ill-formed new command.
     make_command("index create")

quickwit-cli/tests/helpers.rs

@@ -158,7 +158,7 @@ pub enum TestStorageType {
 pub fn create_test_env(index_id: String, storage_type: TestStorageType) -> anyhow::Result<TestEnv> {
     let tempdir = tempdir()?;
     let data_dir_path = tempdir.path().join("data");
-    let indexes_dir_path = tempdir.path().join("indexes");
+    let indexes_dir_path = data_dir_path.join("indexes");
     let resources_dir_path = tempdir.path().join("resources");
 
     for dir_path in [&data_dir_path, &indexes_dir_path, &resources_dir_path] {

quickwit-config/src/config.rs

@@ -43,8 +43,8 @@ fn default_data_dir_path() -> PathBuf {
 // For a given index `index-id`, it means that we have the metastore file
 // in  `./qwdata/indexes/{index-id}/metastore.json` and splits in
 // dir `./qwdata/indexes/{index-id}/splits`.
-fn default_metastore_and_index_root_uri() -> String {
-    Uri::try_new(&default_data_dir_path().join("indexes").to_string_lossy())
+fn default_metastore_and_index_root_uri(data_dir_path: &Path) -> String {
+    Uri::try_new(&data_dir_path.join("indexes").to_string_lossy())
         .expect("Default data dir `./qwdata` value is invalid.")
         .as_ref()
         .to_string()
@@ -150,7 +150,7 @@ pub struct StorageConfig {
     pub s3_config: S3Config,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+#[derive(Clone, Serialize, Deserialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 pub struct QuickwitConfig {
     pub version: usize,
@@ -162,10 +162,8 @@ pub struct QuickwitConfig {
     pub rest_listen_port: u16,
     #[serde(default)]
     pub peer_seeds: Vec<String>,
-    #[serde(default = "default_metastore_and_index_root_uri")]
-    pub metastore_uri: String,
-    #[serde(default = "default_metastore_and_index_root_uri")]
-    pub default_index_root_uri: String,
+    metastore_uri: Option<String>,
+    default_index_root_uri: Option<String>,
     #[serde(default = "default_data_dir_path")]
     #[serde(rename = "data_dir")]
     pub data_dir_path: PathBuf,
@@ -292,6 +290,21 @@ impl QuickwitConfig {
     }
 }
 
+impl QuickwitConfig {
+    pub fn metastore_uri(&self) -> String {
+        self.metastore_uri
+            .as_ref()
+            .unwrap_or(&default_metastore_and_index_root_uri(&self.data_dir_path))
+            .to_string()
+    }
+    pub fn default_index_root_uri(&self) -> String {
+        self.default_index_root_uri
+            .as_ref()
+            .unwrap_or(&default_metastore_and_index_root_uri(&self.data_dir_path))
+            .to_string()
+    }
+}
+
 impl Default for QuickwitConfig {
     fn default() -> Self {
         Self {
@@ -300,8 +313,8 @@ impl Default for QuickwitConfig {
             rest_listen_port: default_rest_listen_port(),
             peer_seeds: Vec::new(),
             node_id: default_node_id(),
-            metastore_uri: default_metastore_and_index_root_uri(),
-            default_index_root_uri: default_metastore_and_index_root_uri(),
+            metastore_uri: None,
+            default_index_root_uri: None,
             data_dir_path: PathBuf::from(DEFAULT_DATA_DIR_PATH),
             indexer_config: IndexerConfig::default(),
             searcher_config: SearcherConfig::default(),
@@ -310,6 +323,25 @@ impl Default for QuickwitConfig {
     }
 }
 
+impl std::fmt::Debug for QuickwitConfig {
+    fn fmt(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+        formatter
+            .debug_struct("QuickwitConfig")
+            .field("version", &self.version)
+            .field("node_id", &self.node_id)
+            .field("listen_address", &self.listen_address)
+            .field("rest_listen_port", &self.rest_listen_port)
+            .field("peer_seeds", &self.peer_seeds)
+            .field("data_dir_path", &self.data_dir_path)
+            .field("metastore_uri", &self.metastore_uri())
+            .field("default_index_root_uri", &self.default_index_root_uri())
+            .field("indexer_config", &self.indexer_config)
+            .field("searcher_config", &self.searcher_config)
+            .field("storage_config", &self.storage_config)
+            .finish()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::env;
@@ -349,7 +381,7 @@ mod tests {
                     ]
                 );
                 assert_eq!(
-                    config.metastore_uri,
+                    config.metastore_uri(),
                     "postgres://username:password@host:port/db"
                 );
 
@@ -410,7 +442,7 @@ mod tests {
             assert_eq!(config.version, 0);
             assert_eq!(config.node_id, "1");
             assert_eq!(
-                config.metastore_uri,
+                config.metastore_uri(),
                 "postgres://username:password@host:port/db"
             );
             assert!(config.storage_config.is_none());
@@ -424,7 +456,7 @@ mod tests {
             let config = serde_yaml::from_str::<QuickwitConfig>(config_yaml).unwrap();
             assert_eq!(config.version, 0);
             assert_eq!(
-                config.metastore_uri,
+                config.metastore_uri(),
                 "postgres://username:password@host:port/db"
             );
             assert_eq!(
@@ -446,7 +478,7 @@ mod tests {
             assert_eq!(config.version, 0);
             assert!(config.node_id.starts_with("node-"));
             assert_eq!(
-                config.metastore_uri,
+                config.metastore_uri(),
                 format!(
                     "file://{}/qwdata/indexes",
                     env::current_dir().unwrap().display()