Index swapping when meilisearch reindex (#853)

* cycling indices * removed printlns * uses swap indices instead * Bring back deletion * Fix tests * Fix version deletion --------- Co-authored-by: Jai A <[email protected]>
modrinth · Jan 13, 2024 · 0aebf37 · 0aebf37
1 parent d1a09d0
commit 0aebf37
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 17 deletions.
diff --git a/src/routes/v2/projects.rs b/src/routes/v2/projects.rs
@@ -384,7 +384,7 @@ pub async fn project_edit(
     req: HttpRequest,
     info: web::Path<(String,)>,
     pool: web::Data<PgPool>,
-    config: web::Data<SearchConfig>,
+    search_config: web::Data<SearchConfig>,
     new_project: web::Json<EditProject>,
     redis: web::Data<RedisPool>,
     session_queue: web::Data<AuthQueue>,
@@ -490,7 +490,7 @@ pub async fn project_edit(
         req.clone(),
         info,
         pool.clone(),
-        config,
+        search_config,
         web::Json(new_project),
         redis.clone(),
         session_queue.clone(),
@@ -865,11 +865,11 @@ pub async fn project_delete(
     info: web::Path<(String,)>,
     pool: web::Data<PgPool>,
     redis: web::Data<RedisPool>,
-    config: web::Data<SearchConfig>,
+    search_config: web::Data<SearchConfig>,
     session_queue: web::Data<AuthQueue>,
 ) -> Result<HttpResponse, ApiError> {
     // Returns NoContent, so no need to convert
-    v3::projects::project_delete(req, info, pool, redis, config, session_queue)
+    v3::projects::project_delete(req, info, pool, redis, search_config, session_queue)
         .await
         .or_else(v2_reroute::flatten_404_error)
 }

diff --git a/src/routes/v3/versions.rs b/src/routes/v3/versions.rs
@@ -838,7 +838,7 @@ pub async fn version_list(
 
 pub async fn version_delete(
     req: HttpRequest,
-    info: web::Path<(models::ids::VersionId,)>,
+    info: web::Path<(VersionId,)>,
     pool: web::Data<PgPool>,
     redis: web::Data<RedisPool>,
     session_queue: web::Data<AuthQueue>,

diff --git a/src/search/indexing/mod.rs b/src/search/indexing/mod.rs
@@ -2,6 +2,7 @@
 pub mod local_import;
 
 use itertools::Itertools;
+use meilisearch_sdk::SwapIndexes;
 use std::collections::HashMap;
 
 use crate::database::redis::RedisPool;
@@ -45,7 +46,9 @@ pub async fn remove_documents(
     ids: &[crate::models::ids::VersionId],
     config: &SearchConfig,
 ) -> Result<(), meilisearch_sdk::errors::Error> {
-    let indexes = get_indexes(config).await?;
+    let mut indexes = get_indexes_for_indexing(config, false).await?;
+    let mut indexes_next = get_indexes_for_indexing(config, true).await?;
+    indexes.append(&mut indexes_next);
 
     for index in indexes {
         index
@@ -63,7 +66,16 @@ pub async fn index_projects(
 ) -> Result<(), IndexingError> {
     info!("Indexing projects.");
 
-    let indices = get_indexes(config).await?;
+    // First, ensure current index exists (so no error happens- current index should be worst-case empty, not missing)
+    get_indexes_for_indexing(config, false).await?;
+
+    // Then, delete the next index if it still exists
+    let indices = get_indexes_for_indexing(config, true).await?;
+    for index in indices {
+        index.delete().await?;
+    }
+    // Recreate the next index for indexing
+    let indices = get_indexes_for_indexing(config, true).await?;
 
     let all_loader_fields =
         crate::database::models::loader_fields::LoaderField::get_fields_all(&pool, &redis)
@@ -75,7 +87,6 @@ pub async fn index_projects(
     let all_ids = get_all_ids(pool.clone()).await?;
     let all_ids_len = all_ids.len();
     info!("Got all ids, indexing {} projects", all_ids_len);
-
     let mut so_far = 0;
     let as_chunks: Vec<_> = all_ids
         .into_iter()
@@ -106,16 +117,42 @@ pub async fn index_projects(
         add_projects(&indices, uploads, all_loader_fields.clone(), config).await?;
     }
 
+    // Swap the index
+    swap_index(config, "projects").await?;
+    swap_index(config, "projects_filtered").await?;
+
+    // Delete the now-old index
+    for index in indices {
+        index.delete().await?;
+    }
+
     info!("Done adding projects.");
     Ok(())
 }
 
-pub async fn get_indexes(
+pub async fn swap_index(config: &SearchConfig, index_name: &str) -> Result<(), IndexingError> {
+    let client = config.make_client();
+    let index_name_next = config.get_index_name(index_name, true);
+    let index_name = config.get_index_name(index_name, false);
+    let swap_indices = SwapIndexes {
+        indexes: (index_name_next, index_name),
+    };
+    client
+        .swap_indexes([&swap_indices])
+        .await?
+        .wait_for_completion(&client, None, Some(TIMEOUT))
+        .await?;
+
+    Ok(())
+}
+
+pub async fn get_indexes_for_indexing(
     config: &SearchConfig,
+    next: bool, // Get the 'next' one
 ) -> Result<Vec<Index>, meilisearch_sdk::errors::Error> {
     let client = config.make_client();
-    let project_name = config.get_index_name("projects");
-    let project_filtered_name = config.get_index_name("projects_filtered");
+    let project_name = config.get_index_name("projects", next);
+    let project_filtered_name = config.get_index_name("projects_filtered", next);
     let projects_index = create_or_update_index(&client, &project_name, None).await?;
     let projects_filtered_index = create_or_update_index(
         &client,
@@ -139,7 +176,7 @@ async fn create_or_update_index(
     name: &str,
     custom_rules: Option<&'static [&'static str]>,
 ) -> Result<Index, meilisearch_sdk::errors::Error> {
-    info!("Updating/creating index.");
+    info!("Updating/creating index {}", name);
 
     match client.get_index(name).await {
         Ok(index) => {

diff --git a/src/search/mod.rs b/src/search/mod.rs
@@ -58,7 +58,7 @@ impl actix_web::ResponseError for SearchError {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Debug, Clone)]
 pub struct SearchConfig {
     pub address: String,
     pub key: String,
@@ -83,8 +83,10 @@ impl SearchConfig {
         Client::new(self.address.as_str(), Some(self.key.as_str()))
     }
 
-    pub fn get_index_name(&self, index: &str) -> String {
-        format!("{}_{}", self.meta_namespace, index)
+    // Next: true if we want the next index (we are preparing the next swap), false if we want the current index (searching)
+    pub fn get_index_name(&self, index: &str, next: bool) -> String {
+        let alt = if next { "_alt" } else { "" };
+        format!("{}_{}_{}", self.meta_namespace, index, alt)
     }
 }
 
@@ -195,8 +197,8 @@ pub fn get_sort_index(
     config: &SearchConfig,
     index: &str,
 ) -> Result<(String, [&'static str; 1]), SearchError> {
-    let projects_name = config.get_index_name("projects");
-    let projects_filtered_name = config.get_index_name("projects_filtered");
+    let projects_name = config.get_index_name("projects", false);
+    let projects_filtered_name = config.get_index_name("projects_filtered", false);
     Ok(match index {
         "relevance" => (projects_name, ["downloads:desc"]),
         "downloads" => (projects_filtered_name, ["downloads:desc"]),

diff --git a/tests/search.rs b/tests/search.rs
@@ -1,3 +1,4 @@
+use actix_http::StatusCode;
 use common::api_v3::ApiV3;
 use common::database::*;
 
@@ -9,6 +10,9 @@ use common::search::setup_search_projects;
 use futures::stream::StreamExt;
 use serde_json::json;
 
+use crate::common::api_common::Api;
+use crate::common::api_common::ApiProject;
+
 mod common;
 
 // TODO: Revisit this wit   h the new modify_json in the version maker
@@ -113,3 +117,52 @@ async fn search_projects() {
     })
     .await;
 }
+
+#[actix_rt::test]
+async fn index_swaps() {
+    with_test_environment(Some(10), |test_env: TestEnvironment<ApiV3>| async move {
+        // Reindex
+        let resp = test_env.api.reset_search_index().await;
+        assert_status!(&resp, StatusCode::NO_CONTENT);
+
+        // Now we should get results
+        let projects = test_env
+            .api
+            .search_deserialized(None, Some(json!([["categories:fabric"]])), USER_USER_PAT)
+            .await;
+        assert_eq!(projects.total_hits, 1);
+        assert!(projects.hits[0].slug.as_ref().unwrap().contains("alpha"));
+
+        // Delete the project
+        let resp = test_env.api.remove_project("alpha", USER_USER_PAT).await;
+        assert_status!(&resp, StatusCode::NO_CONTENT);
+
+        // We should not get any results, because the project has been deleted
+        let projects = test_env
+            .api
+            .search_deserialized(None, Some(json!([["categories:fabric"]])), USER_USER_PAT)
+            .await;
+        assert_eq!(projects.total_hits, 0);
+
+        // But when we reindex, it should be gone
+        let resp = test_env.api.reset_search_index().await;
+        assert_status!(&resp, StatusCode::NO_CONTENT);
+
+        let projects = test_env
+            .api
+            .search_deserialized(None, Some(json!([["categories:fabric"]])), USER_USER_PAT)
+            .await;
+        assert_eq!(projects.total_hits, 0);
+
+        // Reindex again, should still be gone
+        let resp = test_env.api.reset_search_index().await;
+        assert_status!(&resp, StatusCode::NO_CONTENT);
+
+        let projects = test_env
+            .api
+            .search_deserialized(None, Some(json!([["categories:fabric"]])), USER_USER_PAT)
+            .await;
+        assert_eq!(projects.total_hits, 0);
+    })
+    .await;
+}