Replace IndexUid::new with IndexUid::from in metastre shard API impl

`IndexUid::new` sets the incarnation ID to `Ulid::Nil()`.
quickwit-oss · Oct 26, 2023 · 4bd7d67 · 4bd7d67
1 parent 9d78a5c
commit 4bd7d67
Show file tree

Hide file tree

Showing 17 changed files with 4,311 additions and 4,147 deletions.
diff --git a/quickwit/quickwit-cli/tests/cli.rs b/quickwit/quickwit-cli/tests/cli.rs
@@ -684,7 +684,7 @@ async fn test_garbage_collect_cli_no_grace() {
     let split_ids = vec![splits[0].split_id().to_string()];
     let mut metastore = refresh_metastore(metastore).await.unwrap();
     let mark_for_deletion_request =
-        MarkSplitsForDeletionRequest::new(index_uid.to_string(), split_ids.clone());
+        MarkSplitsForDeletionRequest::new(index_uid.clone(), split_ids.clone());
     metastore
         .mark_splits_for_deletion(mark_for_deletion_request)
         .await
@@ -808,7 +808,7 @@ async fn test_garbage_collect_index_cli() {
     let split = splits[0].clone();
     metastore
         .mark_splits_for_deletion(MarkSplitsForDeletionRequest::new(
-            index_uid.to_string(),
+            index_uid.clone(),
             vec![split.split_metadata.split_id.to_string()],
         ))
         .await
@@ -817,8 +817,8 @@ async fn test_garbage_collect_index_cli() {
         .delete_splits(DeleteSplitsRequest {
             index_uid: index_uid.to_string(),
             split_ids: splits
-                .iter()
-                .map(|split| split.split_metadata.split_id.to_string())
+                .into_iter()
+                .map(|split| split.split_metadata.split_id)
                 .collect(),
         })
         .await

diff --git a/quickwit/quickwit-index-management/src/garbage_collection.rs b/quickwit/quickwit-index-management/src/garbage_collection.rs
@@ -138,7 +138,7 @@ pub async fn run_garbage_collect(
         .collect();
     if !split_ids.is_empty() {
         let mark_splits_for_deletion_request =
-            MarkSplitsForDeletionRequest::new(index_uid.to_string(), split_ids);
+            MarkSplitsForDeletionRequest::new(index_uid.clone(), split_ids);
         protect_future(
             progress_opt,
             metastore.mark_splits_for_deletion(mark_splits_for_deletion_request),
@@ -482,7 +482,7 @@ mod tests {
             StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata).unwrap();
         metastore.stage_splits(stage_splits_request).await.unwrap();
         let mark_splits_for_deletion_request =
-            MarkSplitsForDeletionRequest::new(index_uid.to_string(), vec![split_id.to_string()]);
+            MarkSplitsForDeletionRequest::new(index_uid.clone(), vec![split_id.to_string()]);
         metastore
             .mark_splits_for_deletion(mark_splits_for_deletion_request)
             .await
@@ -605,7 +605,7 @@ mod tests {
                 .unwrap();
         metastore.stage_splits(stage_splits_request).await.unwrap();
         let mark_splits_for_deletion =
-            MarkSplitsForDeletionRequest::new(index_uid.to_string(), vec![split_id.to_string()]);
+            MarkSplitsForDeletionRequest::new(index_uid.clone(), vec![split_id.to_string()]);
         metastore
             .mark_splits_for_deletion(mark_splits_for_deletion)
             .await
@@ -713,7 +713,7 @@ mod tests {
         .unwrap();
         metastore.stage_splits(stage_splits_request).await.unwrap();
         let mark_splits_for_deletion_request = MarkSplitsForDeletionRequest::new(
-            index_uid.to_string(),
+            index_uid.clone(),
             vec![split_id_0.to_string(), split_id_1.to_string()],
         );
         metastore

diff --git a/quickwit/quickwit-index-management/src/index.rs b/quickwit/quickwit-index-management/src/index.rs
@@ -194,7 +194,7 @@ impl IndexService {
             .await?
             .deserialize_split_ids()?;
         let mark_splits_for_deletion_request =
-            MarkSplitsForDeletionRequest::new(index_uid.to_string(), split_ids);
+            MarkSplitsForDeletionRequest::new(index_uid.clone(), split_ids);
         self.metastore
             .mark_splits_for_deletion(mark_splits_for_deletion_request)
             .await?;
@@ -297,7 +297,7 @@ impl IndexService {
             .map(|split| split.split_id.to_string())
             .collect();
         let mark_splits_for_deletion_request =
-            MarkSplitsForDeletionRequest::new(index_uid.to_string(), split_ids.clone());
+            MarkSplitsForDeletionRequest::new(index_uid.clone(), split_ids.clone());
         self.metastore
             .mark_splits_for_deletion(mark_splits_for_deletion_request)
             .await?;

diff --git a/quickwit/quickwit-indexing/src/actors/merge_executor.rs b/quickwit/quickwit-indexing/src/actors/merge_executor.rs
@@ -338,7 +338,7 @@ impl MergeExecutor {
         ctx: &ActorContext<Self>,
     ) -> anyhow::Result<Option<IndexedSplit>> {
         let list_delete_tasks_request =
-            ListDeleteTasksRequest::new(split.index_uid.to_string(), split.delete_opstamp);
+            ListDeleteTasksRequest::new(split.index_uid.clone(), split.delete_opstamp);
         let delete_tasks = ctx
             .protect_future(self.metastore.list_delete_tasks(list_delete_tasks_request))
             .await?
@@ -393,8 +393,8 @@ impl MergeExecutor {
                     split.split_id()
                 );
                 let mark_splits_for_deletion_request = MarkSplitsForDeletionRequest::new(
-                    split.index_uid.to_string(),
-                    vec![split.split_id.to_string()],
+                    split.index_uid.clone(),
+                    vec![split.split_id.clone()],
                 );
                 self.metastore
                     .mark_splits_for_deletion(mark_splits_for_deletion_request)

diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs b/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs
@@ -225,7 +225,7 @@ impl DeleteTaskPlanner {
 
         for stale_split in stale_splits {
             let list_delete_tasks_request = ListDeleteTasksRequest::new(
-                self.index_uid.to_string(),
+                self.index_uid.clone(),
                 stale_split.split_metadata.delete_opstamp,
             );
             let pending_tasks = ctx

diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_service.rs b/quickwit/quickwit-janitor/src/actors/delete_task_service.rs
@@ -273,7 +273,7 @@ mod tests {
         // Just test creation of delete query.
         assert_eq!(
             metastore
-                .list_delete_tasks(ListDeleteTasksRequest::new(index_uid.to_string(), 0))
+                .list_delete_tasks(ListDeleteTasksRequest::new(index_uid.clone(), 0))
                 .await
                 .unwrap()
                 .delete_tasks

diff --git a/quickwit/quickwit-janitor/src/retention_policy_execution.rs b/quickwit/quickwit-janitor/src/retention_policy_execution.rs
@@ -90,7 +90,7 @@ pub async fn run_execute_retention_policy(
         expired_split_ids.len()
     );
     let mark_splits_for_deletion_request =
-        MarkSplitsForDeletionRequest::new(index_uid.to_string(), expired_split_ids);
+        MarkSplitsForDeletionRequest::new(index_uid, expired_split_ids);
     ctx.protect_future(metastore.mark_splits_for_deletion(mark_splits_for_deletion_request))
         .await?;
     Ok(expired_splits)

diff --git a/quickwit/quickwit-metastore/src/lib.rs b/quickwit/quickwit-metastore/src/lib.rs
@@ -28,8 +28,6 @@
 //! - PostgreSQL metastore
 //! etc.
 
-#[macro_use]
-mod tests;
 #[allow(missing_docs)]
 pub mod checkpoint;
 mod error;
@@ -38,6 +36,8 @@ mod metastore_factory;
 mod metastore_resolver;
 mod split_metadata;
 mod split_metadata_version;
+#[cfg(test)]
+pub(crate) mod tests;
 
 use std::ops::Range;
 
@@ -100,21 +100,9 @@ pub fn split_tag_filter(
 mod backward_compatibility_tests;
 
 #[cfg(any(test, feature = "testsuite"))]
-mod for_test {
-    use std::sync::Arc;
-
-    use quickwit_proto::metastore::MetastoreServiceClient;
-    use quickwit_storage::RamStorage;
-
-    use super::FileBackedMetastore;
-
-    /// Returns a metastore backed by an "in-memory file" for testing.
-    pub fn metastore_for_test() -> MetastoreServiceClient {
-        MetastoreServiceClient::new(FileBackedMetastore::for_test(Arc::new(
-            RamStorage::default(),
-        )))
-    }
+/// Returns a metastore backed by an "in-memory file" for testing.
+pub fn metastore_for_test() -> quickwit_proto::metastore::MetastoreServiceClient {
+    quickwit_proto::metastore::MetastoreServiceClient::new(FileBackedMetastore::for_test(
+        std::sync::Arc::new(quickwit_storage::RamStorage::default()),
+    ))
 }
-
-#[cfg(any(test, feature = "testsuite"))]
-pub use for_test::metastore_for_test;
diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/shards.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/shards.rs
@@ -387,9 +387,6 @@ mod tests {
         assert_eq!(shards.shards.get(&2).unwrap(), shard);
     }
 
-    #[test]
-    fn test_close_shard() {}
-
     #[test]
     fn test_list_shards() {
         let index_uid: IndexUid = "test-index:0".into();

diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs
@@ -723,7 +723,7 @@ impl MetastoreService for FileBackedMetastore {
         let grouped_subrequests: HashMap<IndexUid, Vec<OpenShardsSubrequest>> = request
             .subrequests
             .into_iter()
-            .into_group_map_by(|subrequest| IndexUid::new(subrequest.index_uid.clone()));
+            .into_group_map_by(|subrequest| IndexUid::from(subrequest.index_uid.clone()));
 
         for (index_uid, subrequests) in grouped_subrequests {
             let subresponses = self
@@ -746,7 +746,7 @@ impl MetastoreService for FileBackedMetastore {
         let grouped_subrequests: HashMap<IndexUid, Vec<AcquireShardsSubrequest>> = request
             .subrequests
             .into_iter()
-            .into_group_map_by(|subrequest| IndexUid::new(subrequest.index_uid.clone()));
+            .into_group_map_by(|subrequest| IndexUid::from(subrequest.index_uid.clone()));
 
         for (index_uid, subrequests) in grouped_subrequests {
             let subresponses = self
@@ -768,7 +768,7 @@ impl MetastoreService for FileBackedMetastore {
         let grouped_subrequests: HashMap<IndexUid, Vec<DeleteShardsSubrequest>> = request
             .subrequests
             .into_iter()
-            .into_group_map_by(|subrequest| IndexUid::new(subrequest.index_uid.clone()));
+            .into_group_map_by(|subrequest| IndexUid::from(subrequest.index_uid.clone()));
 
         for (index_uid, subrequests) in grouped_subrequests {
             let subresponse = self
@@ -943,7 +943,7 @@ fn build_regex_exprs_from_pattern(index_pattern: &str) -> anyhow::Result<String>
 
 #[cfg(test)]
 #[async_trait]
-impl crate::tests::test_suite::DefaultForTest for FileBackedMetastore {
+impl crate::tests::DefaultForTest for FileBackedMetastore {
     async fn default_for_test() -> Self {
         use quickwit_storage::RamStorage;
         FileBackedMetastore::try_new(Arc::new(RamStorage::default()), None)
@@ -952,8 +952,6 @@ impl crate::tests::test_suite::DefaultForTest for FileBackedMetastore {
     }
 }
 
-metastore_test_suite!(crate::FileBackedMetastore);
-
 #[cfg(test)]
 mod tests {
     use std::collections::HashMap;
@@ -975,8 +973,10 @@ mod tests {
         fetch_or_init_indexes_states, meta_path, put_index_given_index_id, put_indexes_states,
     };
     use super::*;
-    use crate::tests::test_suite::DefaultForTest;
-    use crate::{IndexMetadata, ListSplitsQuery, SplitMetadata, SplitState};
+    use crate::tests::DefaultForTest;
+    use crate::{metastore_test_suite, IndexMetadata, ListSplitsQuery, SplitMetadata, SplitState};
+
+    metastore_test_suite!(crate::FileBackedMetastore);
 
     #[tokio::test]
     async fn test_metastore_connectivity_and_endpoints() {

diff --git a/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs b/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs
@@ -1482,7 +1482,7 @@ impl MetastoreFactory for PostgresqlMetastoreFactory {
 
 #[cfg(test)]
 #[async_trait]
-impl crate::tests::test_suite::DefaultForTest for PostgresqlMetastore {
+impl crate::tests::DefaultForTest for PostgresqlMetastore {
     async fn default_for_test() -> Self {
         // We cannot use a singleton here,
         // because sqlx needs the runtime used to create a connection to
@@ -1507,8 +1507,6 @@ impl crate::tests::test_suite::DefaultForTest for PostgresqlMetastore {
     }
 }
 
-metastore_test_suite!(crate::PostgresqlMetastore);
-
 #[cfg(test)]
 mod tests {
     use quickwit_doc_mapper::tag_pruning::{no_tag, tag, TagFilterAst};
@@ -1518,8 +1516,10 @@ mod tests {
 
     use super::{build_query_filter, tags_filter_expression_helper, PostgresqlMetastore};
     use crate::metastore::postgresql_metastore::build_index_id_patterns_sql_query;
-    use crate::tests::test_suite::DefaultForTest;
-    use crate::{ListSplitsQuery, SplitState};
+    use crate::tests::DefaultForTest;
+    use crate::{metastore_test_suite, ListSplitsQuery, SplitState};
+
+    metastore_test_suite!(crate::PostgresqlMetastore);
 
     #[tokio::test]
     async fn test_metastore_connectivity_and_endpoints() {