tests: Faster search::multi IT tests #5603

martin-g · 2025-05-29T13:18:02Z

Pull Request

Related issue

What does this PR do?

Use shared server + unique indices where possible

martin-g · 2025-05-29T13:20:47Z

@irevoire This one is huge! Please take a look when you have few minutes and let me know if you have any concerns!
One thing that bothers me a bit is that now the snapshots use [uuid] for all index names and it is not very clear which index brings the hit, e.g. see federation_inconsistent_merge_order() test case.

irevoire · 2025-06-03T10:07:39Z

One thing that bothers me a bit is that now the snapshots use [uuid] for all index names and it is not very clear which index brings the hit, e.g. see federation_inconsistent_merge_order() test case.

Hey yeah, you're right, that's really hard to read 😩

In the beginning, you are using a lot of shared indices, and since these ones have a fixed name, I find the test pretty good to read.
From what I see, almost all the tests rely on like five different datasets (and some are already available as shared index). Do you think it would make sense to make all of these datasets into a unique, named and shared index for all test?

And if a test must update the settings or something, then we don't use the shared server?
It's not ideal, but I feel like it would fix the issue for 99% of the tests while keeping the test very easy to read (maybe even more than before since we won't have to repeat all the initialization work)

Use shared server + unique indices where possible Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

It could be used when we want to see the index name in the assertions, e.g. `movies-[uuid]` Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g · 2025-06-10T11:51:44Z

@irevoire 1824fbd introduces Index::unique_index_with_prefix(&str)
Using it produces index names like movies-[uuid] in the response JSON. This way the index is both unique and readable.

irevoire · 2025-06-10T12:10:30Z

Oh yes that's easy and smart, I love it 🔥

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

…` counterparts Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g · 2025-06-11T05:57:00Z

@irevoire I think this big boy is ready for review!
You may make yourself some🍿 ! 😄

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g · 2025-06-11T06:29:11Z

One of the tests fails due to racing:

---- search::multi::search_multiple_indexes_dont_exist stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot: search_multiple_indexes_dont_exist
Source: crates/meilisearch/tests/search/multi/mod.rs:794
────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────
    0     0 │ {
    1       │-  "message": "Inside `.queries[0]`: Index `test` not found.",
          1 │+  "message": "Inside `.queries[1]`: Index `nested` not found.",
    2     2 │   "code": "index_not_found",
    3     3 │   "type": "invalid_request",
    4     4 │   "link": "https://docs.meilisearch.com/errors#index_not_found"
    5     5 │ }

Both test and nested indices do not exist in the shared server, but sometimes the error is about the first index (test) and sometimes for the second one (nested)...

Update: It looks like the test index is actually existing in the shared server. Using unique index names consistently returns errors with the first index name in the details.

By using hardcoded there is a chance that the index could exist Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

irevoire

Hey!

I saw this chunk of code literally dozens of times I think:

    let documents = DOCUMENTS.clone();
    let (value, _) = movies_index.add_documents(documents, None).await;
    movies_index.wait_task(value.uid()).await.succeeded();

    let (value, _) = movies_index
        .update_settings(json!({
          "sortableAttributes": ["title"],
          "filterableAttributes": ["title", "color"],
          "rankingRules": [
            "sort",
            "words",
            "typo",
            "proximity",
            "attribute",
            "exactness"
          ]
        }))
        .await;
    movies_index.wait_task(value.uid()).await.succeeded();

    let batman_index = server.unique_index_with_prefix("batman");

    let documents = SCORE_DOCUMENTS.clone();
    let (value, _) = batman_index.add_documents(documents, None).await;
    batman_index.wait_task(value.uid()).await.succeeded();

    let (value, _) = batman_index
        .update_settings(json!({
          "sortableAttributes": ["title"],
          "filterableAttributes": ["title"],
          "rankingRules": [
            "sort",
            "words",
            "typo",
            "proximity",
            "attribute",
            "exactness"
          ]
        }))
        .await;
    batman_index.wait_task(value.uid()).await.succeeded();

I think it would make sense to create two new shared index for these two datasets 🤔

Since playing with the ranking rules seems very specific to the multisearch and federation I think they could be local to this file though (at the top would be the best)

Otherwise it looks pretty good! I'll take a look at your issue right now

crates/meilisearch/tests/search/multi/mod.rs

irevoire · 2025-06-11T09:32:13Z

crates/meilisearch/tests/search/multi/mod.rs

+    let index_1 = format!("index_1-{}", Uuid::new_v4());
+    let index_2 = format!("index_2-{}", Uuid::new_v4());


Why are we not using the unique_with_prefix as you suggested?
It seems "safer" to use

Initially I used server.unique_index_with_prefix("index_1") but then I simplified it to plain strings because server.unique_index() looks like it is creating the index. Using a plain string makes it more obvious that this is just a name.
I don't mind changing it back to server.unique_index_with_prefix() if you think it would be better!

irevoire · 2025-06-11T09:47:23Z

crates/meilisearch/tests/search/multi/mod.rs

-        {"indexUid" : "test", "q": "glass"},
-        {"indexUid": "nested", "q": "pésti", "sort": ["doggos:desc"]},
+        {"indexUid" : index.uid, "q": "glass"},
+        {"indexUid": nested_index.uid, "q": "pésti", "sort": ["mother:desc"]},


crates/meilisearch/tests/search/multi/mod.rs

irevoire · 2025-06-11T09:51:58Z

crates/meilisearch/tests/search/multi/mod.rs

+    let batman_index = server.unique_index_with_prefix("batman");

    let documents = SCORE_DOCUMENTS.clone();
-    let (value, _) = index.add_documents(documents, None).await;
-    index.wait_task(value.uid()).await.succeeded();
+    let (value, _) = batman_index.add_documents(documents, None).await;
+    batman_index.wait_task(value.uid()).await.succeeded();

-    let (value, _) = index
+    let (value, _) = batman_index


From what I see here we're using the default ranking rules and could use the shared index

https://www.meilisearch.com/docs/learn/relevancy/ranking_rules#list-of-built-in-ranking-rules

irevoire · 2025-06-11T09:52:37Z

crates/meilisearch/tests/search/multi/mod.rs

+    let (value, _) = movies_index.add_documents(documents, None).await;
+    movies_index.wait_task(value.uid()).await.succeeded();

-    let (value, _) = index
+    let (value, _) = movies_index


Same question

Here the order of the ranking rules is not the same as the default ones.
Here:

"rankingRules": [ "sort", "words", "typo", "proximity", "attribute", "exactness" ]

Defaults:

[ "words", "typo", "proximity", "attribute", "sort", "exactness" ]

irevoire · 2025-06-11T09:52:47Z

crates/meilisearch/tests/search/multi/mod.rs

    let documents = SCORE_DOCUMENTS.clone();
-    let (value, _) = index.add_documents(documents, None).await;
-    index.wait_task(value.uid()).await.succeeded();
+    let (value, _) = batman_index.add_documents(documents, None).await;
+    batman_index.wait_task(value.uid()).await.succeeded();

-    let (value, _) = index
+    let (value, _) = batman_index


Same question

Here the order of the ranking rules is not the same as the default ones.
Here:

"rankingRules": [ "sort", "words", "typo", "proximity", "attribute", "exactness" ]

Defaults:

[ "words", "typo", "proximity", "attribute", "sort", "exactness" ]

irevoire · 2025-06-11T10:05:39Z

Update: It looks like the test index is actually existing in the shared server. Using unique index names consistently returns errors with the first index name in the details.

Ah I see you found the bug while I was reviewing your PR.
But oooof that's a big bug we're never supposed to be able to create a non-unique index with the shared index and that could come from any test 🤔
Maybe a quick way to find the issue would be to update the index creation call to crash when it's called with an index name that contains test?

…s the default Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g · 2025-06-12T06:30:36Z

But oooof that's a big bug we're never supposed to be able to create a non-unique index with the shared index and that could come from any test

I just tried to list all indexes in the shared server and confirm my assumption above with:

let (response, _code) = server.list_indexes(Some(0), Some(1_000_000)).await;
    dbg!(response);

but it returns:

       START             meilisearch::integration search::multi::search_multiple_indexes_dont_exist

running 1 test
[crates/meilisearch/tests/search/multi/mod.rs:792:5] response = Value(
    Object {
        "results": Array [],
        "offset": Number(0),
        "limit": Number(1000000),
        "total": Number(0),
    },
)

The shared server says "I have no indexes"!

And this is expected since every IT test setups its own Actix-Web App!
So my explanation above is not correct!
It should be a racing issue in the backend ...

martin-g · 2025-06-12T08:43:35Z

I was using cargo nextest and this caused the problem with the empty list of indexes!
Using cargo test shows a list with many entries but all of the items are either unique or shared. All looks good!

martin-g · 2025-06-12T08:58:11Z

AFAIS the queries are processed in the order from the request body JSON and there is no racing -

meilisearch/crates/meilisearch/src/routes/multi_search.rs

Lines 214 to 240 in aefebde

    
           for (query_index, (index_uid, query, federation_options)) in queries 
        
               .into_iter() 
        
               .map(SearchQueryWithIndex::into_index_query_federation) 
        
               .enumerate() 
        
           { 
        
               debug!(on_index = query_index, parameters = ?query, "Multi-search"); 
        
               if federation_options.is_some() { 
        
                   return Err(( 
        
                       MeilisearchHttpError::FederationOptionsInNonFederatedRequest( 
        
                           query_index, 
        
                       ) 
        
                       .into(), 
        
                       query_index, 
        
                   )); 
        
               } 
        
               let index = index_scheduler 
        
                   .index(&index_uid) 
        
                   .map_err(|err| { 
        
                       let mut err = ResponseError::from(err); 
        
                       // Patch the HTTP status code to 400 as it defaults to 404 for `index_not_found`, but 
        
                       // here the resource not found is not part of the URL. 
        
                       err.code = StatusCode::BAD_REQUEST; 
        
                       err 
        
                   }) 
        
                   .with_index(query_index)?;

Mistery!

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

…anually with Uuid Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

…erver Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g · 2025-06-14T12:07:08Z

@irevoire Here is the "test" in a shared server -

meilisearch/crates/meilisearch/tests/index/create_index.rs

Line 50 in c3368e6

"uid": "test",

Noice! :-)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

irevoire · 2025-06-16T09:41:19Z

Noice! :-)

Oof good catch!

That's why I feels like even if we're not creating the indexes we should always just use the method that makes them unique ahah

irevoire

Awesome, you removed so many indexing process with this file 🎉

tests: Faster search::multi IT tests

8fa6e86

Use shared server + unique indices where possible Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g force-pushed the faster-search-multi-it-tests branch from ac0b945 to 8fa6e86 Compare June 10, 2025 11:10

martin-g added 2 commits June 10, 2025 14:48

Fix typos in comments and update assertions

34d8a54

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Introduce Index::unique_index_with_prefix(&str)

1824fbd

It could be used when we want to see the index name in the assertions, e.g. `movies-[uuid]` Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g added 4 commits June 10, 2025 16:58

More fixes of the tests

6a68397

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

More assertion fixes

8a916a4

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

More assertion fixes

0263eb0

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Remove useless dynamic redactions. They are covered by their `.**.xyz…

bb4baf7

…` counterparts Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g marked this pull request as ready for review June 11, 2025 05:54

Formatting

824f5b1

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Make the dynamic assertion for facetsByIndex JSON key more broader

a73d3c0

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g added 2 commits June 11, 2025 11:01

Use unique indices for the searches in non-existing indices

620867d

By using hardcoded there is a chance that the index could exist Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Sort the imports

b8845d1

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

irevoire requested changes Jun 11, 2025

View reviewed changes

Re-use the shared_index_with_score_documents since the settings are a…

646e44d

…s the default Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g added 3 commits June 12, 2025 13:46

Extract shared indices for movies and batman documents

e8774ad

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Use unique_index_with_prefix() instead of composing the index names m…

2269104

…anually with Uuid Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Try to debug the problem with the existing "test" index in a shared s…

0598320

…erver Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Use a unique name for an index in a shared server

95e8a9b

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Remove debug leftovers

6ee608c

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

irevoire approved these changes Jun 16, 2025

View reviewed changes

irevoire added maintenance Issue about maintenance (CI, tests, refacto...) no db change The database didn't change labels Jun 16, 2025

irevoire added this to the v1.16.0 milestone Jun 16, 2025

irevoire added this pull request to the merge queue Jun 16, 2025

Merged via the queue into meilisearch:main with commit aeaac72 Jun 16, 2025
14 of 16 checks passed

BrewTestBot mentioned this pull request Aug 4, 2025

meilisearch 1.16.0 Homebrew/homebrew-core#232297

Merged

meili-bot added the v1.16.0 PRs/issues solved in v1.16.0 released on 2025-08-04 label Aug 5, 2025

		let index_1 = format!("index_1-{}", Uuid::new_v4());
		let index_2 = format!("index_2-{}", Uuid::new_v4());

tests: Faster search::multi IT tests #5603

tests: Faster search::multi IT tests #5603

Uh oh!

Conversation

martin-g commented May 29, 2025

Pull Request

Related issue

What does this PR do?

Uh oh!

martin-g commented May 29, 2025

Uh oh!

irevoire commented Jun 3, 2025

Uh oh!

martin-g commented Jun 10, 2025

Uh oh!

irevoire commented Jun 10, 2025

Uh oh!

martin-g commented Jun 11, 2025

Uh oh!

martin-g commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

irevoire left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irevoire commented Jun 11, 2025

Uh oh!

martin-g commented Jun 12, 2025

Uh oh!

martin-g commented Jun 12, 2025

Uh oh!

martin-g commented Jun 12, 2025

Uh oh!

martin-g commented Jun 14, 2025

Uh oh!

irevoire commented Jun 16, 2025

Uh oh!

irevoire left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

martin-g commented Jun 11, 2025 •

edited

Loading