Merge #5058

5058: Retry if deserialization of remote response failed r=irevoire a=dureuill

v1.11.2 introduces a new error condition (instead of a possibly infinite wait): deserializing the body of a remote response can cause a timeout and fail immediately.

This PR makes it so that deserialization failures are inside the retry loop, meaning they obey the exponential backoff retry strategy.

Co-authored-by: Louis Dureuil <louis@meilisearch.com>

Cargo.lock

@@ -472,7 +472,7 @@ checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6"
 
 [[package]]
 name = "benchmarks"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "anyhow",
  "bytes",
@@ -653,7 +653,7 @@ dependencies = [
 
 [[package]]
 name = "build-info"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "anyhow",
  "time",
@@ -1623,7 +1623,7 @@ dependencies = [
 
 [[package]]
 name = "dump"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "anyhow",
  "big_s",
@@ -1835,7 +1835,7 @@ checksum = "9fc0510504f03c51ada170672ac806f1f105a88aa97a5281117e1ddc3368e51a"
 
 [[package]]
 name = "file-store"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "tempfile",
  "thiserror",
@@ -1857,7 +1857,7 @@ dependencies = [
 
 [[package]]
 name = "filter-parser"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "insta",
  "nom",
@@ -1877,7 +1877,7 @@ dependencies = [
 
 [[package]]
 name = "flatten-serde-json"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "criterion",
  "serde_json",
@@ -2001,7 +2001,7 @@ dependencies = [
 
 [[package]]
 name = "fuzzers"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "arbitrary",
  "clap",
@@ -2553,7 +2553,7 @@ checksum = "206ca75c9c03ba3d4ace2460e57b189f39f43de612c2f85836e65c929701bb2d"
 
 [[package]]
 name = "index-scheduler"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "anyhow",
  "arroy",
@@ -2747,7 +2747,7 @@ dependencies = [
 
 [[package]]
 name = "json-depth-checker"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "criterion",
  "serde_json",
@@ -3366,7 +3366,7 @@ checksum = "490cc448043f947bae3cbee9c203358d62dbee0db12107a74be5c30ccfd09771"
 
 [[package]]
 name = "meili-snap"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "insta",
  "md5",
@@ -3375,7 +3375,7 @@ dependencies = [
 
 [[package]]
 name = "meilisearch"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "actix-cors",
  "actix-http",
@@ -3465,7 +3465,7 @@ dependencies = [
 
 [[package]]
 name = "meilisearch-auth"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "base64 0.22.1",
  "enum-iterator",
@@ -3484,7 +3484,7 @@ dependencies = [
 
 [[package]]
 name = "meilisearch-types"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "actix-web",
  "anyhow",
@@ -3514,7 +3514,7 @@ dependencies = [
 
 [[package]]
 name = "meilitool"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "anyhow",
  "clap",
@@ -3545,7 +3545,7 @@ dependencies = [
 
 [[package]]
 name = "milli"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "arroy",
  "big_s",
@@ -3991,7 +3991,7 @@ checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e"
 
 [[package]]
 name = "permissive-json-pointer"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "big_s",
  "serde_json",
@@ -6380,7 +6380,7 @@ dependencies = [
 
 [[package]]
 name = "xtask"
-version = "1.11.2"
+version = "1.11.3"
 dependencies = [
  "anyhow",
  "build-info",

Cargo.toml

@@ -22,7 +22,7 @@ members = [
 ]
 
 [workspace.package]
-version = "1.11.2"
+version = "1.11.3"
 authors = [
     "Quentin de Quelen <quentin@dequelen.me>",
     "Clément Renault <clement@meilisearch.com>",

milli/src/vector/error.rs

@@ -58,7 +58,7 @@ pub enum EmbedErrorKind {
     ManualEmbed(String),
     #[error("model not found. Meilisearch will not automatically download models from the Ollama library, please pull the model manually{}", option_info(.0.as_deref(), "server replied with "))]
     OllamaModelNotFoundError(Option<String>),
-    #[error("error deserialization the response body as JSON:\n  - {0}")]
+    #[error("error deserializing the response body as JSON:\n  - {0}")]
     RestResponseDeserialization(std::io::Error),
     #[error("expected a response containing {0} embeddings, got only {1}")]
     RestResponseEmbeddingCount(usize, usize),

milli/src/vector/rest.rs

@@ -252,12 +252,12 @@ where
 
     for attempt in 0..10 {
         let response = request.clone().send_json(&body);
-        let result = check_response(response, data.configuration_source);
+        let result = check_response(response, data.configuration_source).and_then(|response| {
+            response_to_embedding(response, data, expected_count, expected_dimension)
+        });
 
         let retry_duration = match result {
-            Ok(response) => {
+            Ok(response) => return Ok(response),
-                return response_to_embedding(response, data, expected_count, expected_dimension)
-            }
             Err(retry) => {
                 tracing::warn!("Failed: {}", retry.error);
                 if let Some(deadline) = deadline {
@@ -289,6 +289,7 @@ where
     let result = check_response(response, data.configuration_source);
     result.map_err(Retry::into_error).and_then(|response| {
         response_to_embedding(response, data, expected_count, expected_dimension)
+            .map_err(Retry::into_error)
     })
 }
 
@@ -330,23 +331,28 @@ fn response_to_embedding(
     data: &EmbedderData,
     expected_count: usize,
     expected_dimensions: Option<usize>,
-) -> Result<Vec<Embeddings<f32>>, EmbedError> {
+) -> Result<Vec<Embeddings<f32>>, Retry> {
-    let response: serde_json::Value =
+    let response: serde_json::Value = response
-        response.into_json().map_err(EmbedError::rest_response_deserialization)?;
+        .into_json()
+        .map_err(EmbedError::rest_response_deserialization)
+        .map_err(Retry::retry_later)?;
 
-    let embeddings = data.response.extract_embeddings(response)?;
+    let embeddings = data.response.extract_embeddings(response).map_err(Retry::give_up)?;
 
     if embeddings.len() != expected_count {
-        return Err(EmbedError::rest_response_embedding_count(expected_count, embeddings.len()));
+        return Err(Retry::give_up(EmbedError::rest_response_embedding_count(
+            expected_count,
+            embeddings.len(),
+        )));
     }
 
     if let Some(dimensions) = expected_dimensions {
         for embedding in &embeddings {
             if embedding.dimension() != dimensions {
-                return Err(EmbedError::rest_unexpected_dimension(
+                return Err(Retry::give_up(EmbedError::rest_unexpected_dimension(
                     dimensions,
                     embedding.dimension(),
-                ));
+                )));
             }
         }
     }