Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace IndexUid::new with IndexUid::from in metastore shard API impl #4033

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Replace IndexUid::new with IndexUid::from in metastore shard API impl #4033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions quickwit/quickwit-cli/tests/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions quickwit/quickwit-index-management/src/garbage_collection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions quickwit/quickwit-index-management/src/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand Down Expand Up @@ -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?;
Expand Down
6 changes: 3 additions & 3 deletions quickwit/quickwit-indexing/src/actors/merge_executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion quickwit/quickwit-janitor/src/actors/delete_task_planner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion quickwit/quickwit-janitor/src/actors/delete_task_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion quickwit/quickwit-janitor/src/retention_policy_execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 7 additions & 19 deletions quickwit/quickwit-metastore/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
//! - PostgreSQL metastore
//! etc.

#[macro_use]
mod tests;
#[allow(missing_docs)]
pub mod checkpoint;
mod error;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
3 changes: 0 additions & 3 deletions quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/shards.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
16 changes: 8 additions & 8 deletions quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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};
Expand All @@ -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() {
Expand Down
Loading