index_scheduler.features() is no longer fallible

index-scheduler/src/batch.rs

@@ -896,7 +896,7 @@ impl IndexScheduler {
                 })?;
 
                 // 4. Dump experimental feature settings
-                let features = self.features()?.runtime_features();
+                let features = self.features().runtime_features();
                 dump.create_experimental_features(features)?;
 
                 let dump_uid = started_at.format(format_description!(

index-scheduler/src/error.rs

@@ -113,8 +113,6 @@ pub enum Error {
     Dump(#[from] dump::Error),
     #[error(transparent)]
     Heed(#[from] heed::Error),
-    #[error("Unable to record metrics for request.")]
-    CannotRecordMetrics,
     #[error(transparent)]
     Milli(#[from] milli::Error),
     #[error("An unexpected crash occurred when processing the task.")]
@@ -127,8 +125,6 @@ pub enum Error {
     Persist(#[from] tempfile::PersistError),
     #[error(transparent)]
     FeatureNotEnabled(#[from] FeatureNotEnabledError),
-    #[error("An unexpected error occurred when accessing the runtime features.")]
-    RuntimeFeatureToggleError,
 
     #[error(transparent)]
     Anyhow(#[from] anyhow::Error),
@@ -181,14 +177,12 @@ impl Error {
             | Error::TaskCancelationWithEmptyQuery
             | Error::Dump(_)
             | Error::Heed(_)
-            | Error::CannotRecordMetrics
             | Error::Milli(_)
             | Error::ProcessBatchPanicked
             | Error::FileStore(_)
             | Error::IoError(_)
             | Error::Persist(_)
             | Error::FeatureNotEnabled(_)
-            | Error::RuntimeFeatureToggleError
             | Error::Anyhow(_) => true,
             Error::CreateBatch(_)
             | Error::CorruptedTaskQueue
@@ -229,13 +223,11 @@ impl ErrorCode for Error {
             Error::Milli(e) => e.error_code(),
             Error::ProcessBatchPanicked => Code::Internal,
             Error::Heed(e) => e.error_code(),
-            Error::CannotRecordMetrics => Code::Internal,
             Error::HeedTransaction(e) => e.error_code(),
             Error::FileStore(e) => e.error_code(),
             Error::IoError(e) => e.error_code(),
             Error::Persist(e) => e.error_code(),
             Error::FeatureNotEnabled(_) => Code::FeatureNotEnabled,
-            Error::RuntimeFeatureToggleError => Code::Internal,
 
             // Irrecoverable errors
             Error::Anyhow(_) => Code::Internal,

index-scheduler/src/features.rs

@@ -4,7 +4,6 @@ use meilisearch_types::features::{InstanceTogglableFeatures, RuntimeTogglableFea
 use meilisearch_types::heed::types::{SerdeJson, Str};
 use meilisearch_types::heed::{Database, Env, RwTxn};
 
-use crate::error::Error::RuntimeFeatureToggleError;
 use crate::error::FeatureNotEnabledError;
 use crate::Result;
 
@@ -22,9 +21,9 @@ pub struct RoFeatures {
 }
 
 impl RoFeatures {
-    fn new(data: &FeatureData) -> Result<Self> {
-        let runtime = data.runtime_features()?;
-        Ok(Self { runtime })
+    fn new(data: &FeatureData) -> Self {
+        let runtime = data.runtime_features();
+        Self { runtime }
     }
 
     pub fn runtime_features(&self) -> RuntimeTogglableFeatures {
@@ -109,16 +108,22 @@ impl FeatureData {
         self.persisted.put(&mut wtxn, EXPERIMENTAL_FEATURES, &features)?;
         wtxn.commit()?;
 
-        let mut toggled_features = self.runtime.write().map_err(|_| RuntimeFeatureToggleError)?;
+        // safe to unwrap, the lock will only fail if:
+        // 1. requested by the same thread concurrently -> it is called and released in methods that don't call each other
+        // 2. there's a panic while the thread is held -> it is only used for an assignment here.
+        let mut toggled_features = self.runtime.write().unwrap();
         *toggled_features = features;
         Ok(())
     }
 
-    fn runtime_features(&self) -> Result<RuntimeTogglableFeatures> {
-        Ok(*self.runtime.read().map_err(|_| RuntimeFeatureToggleError)?)
+    fn runtime_features(&self) -> RuntimeTogglableFeatures {
+        // sound to unwrap, the lock will only fail if:
+        // 1. requested by the same thread concurrently -> it is called and released in methods that don't call each other
+        // 2. there's a panic while the thread is held -> it is only used for copying the data here
+        *self.runtime.read().unwrap()
     }
 
-    pub fn features(&self) -> Result<RoFeatures> {
+    pub fn features(&self) -> RoFeatures {
         RoFeatures::new(self)
     }
 }

index-scheduler/src/lib.rs

@@ -579,13 +579,7 @@ impl IndexScheduler {
                 run.wake_up.wait();
 
                 loop {
-                    let puffin_enabled = match run.features() {
-                        Ok(features) => features.check_puffin().is_ok(),
-                        Err(e) => {
-                            log::error!("{e}");
-                            continue;
-                        }
-                    };
+                    let puffin_enabled = run.features().check_puffin().is_ok();
                     puffin::set_scopes_on(puffin_enabled);
                     puffin::GlobalProfiler::lock().new_frame();
 
@@ -1299,7 +1293,7 @@ impl IndexScheduler {
         Ok(IndexStats { is_indexing, inner_stats: index_stats })
     }
 
-    pub fn features(&self) -> Result<RoFeatures> {
+    pub fn features(&self) -> RoFeatures {
         self.features.features()
     }
 

meilisearch/src/middleware.rs

@@ -6,9 +6,7 @@ use actix_web::dev::{self, Service, ServiceRequest, ServiceResponse, Transform};
 use actix_web::web::Data;
 use actix_web::Error;
 use futures_util::future::LocalBoxFuture;
-use index_scheduler::Error::CannotRecordMetrics;
 use index_scheduler::IndexScheduler;
-use meilisearch_types::error::ResponseError;
 use prometheus::HistogramTimer;
 
 pub struct RouteMetrics;
@@ -55,14 +53,7 @@ where
         // calling unwrap here is safe because index scheduler is added to app data while creating actix app.
         // also, the tests will fail if this is not present.
         let index_scheduler = req.app_data::<Data<IndexScheduler>>().unwrap();
-        let features = match index_scheduler.features() {
-            Ok(features) => features,
-            Err(_e) => {
-                return Box::pin(
-                    async move { Err(ResponseError::from(CannotRecordMetrics).into()) },
-                );
-            }
-        };
+        let features = index_scheduler.features();
 
         if features.check_metrics().is_ok() {
             let request_path = req.path();

meilisearch/src/routes/features.rs

@@ -29,12 +29,12 @@ async fn get_features(
     >,
     req: HttpRequest,
     analytics: Data<dyn Analytics>,
-) -> Result<HttpResponse, ResponseError> {
-    let features = index_scheduler.features()?;
+) -> HttpResponse {
+    let features = index_scheduler.features();
 
     analytics.publish("Experimental features Seen".to_string(), json!(null), Some(&req));
     debug!("returns: {:?}", features.runtime_features());
-    Ok(HttpResponse::Ok().json(features.runtime_features()))
+    HttpResponse::Ok().json(features.runtime_features())
 }
 
 #[derive(Debug, Deserr)]
@@ -59,7 +59,7 @@ async fn patch_features(
     req: HttpRequest,
     analytics: Data<dyn Analytics>,
 ) -> Result<HttpResponse, ResponseError> {
-    let features = index_scheduler.features()?;
+    let features = index_scheduler.features();
 
     let old_features = features.runtime_features();
     let new_features = meilisearch_types::features::RuntimeTogglableFeatures {

meilisearch/src/routes/indexes/facet_search.rs

@@ -68,7 +68,7 @@ pub async fn search(
     }
 
     let index = index_scheduler.index(&index_uid)?;
-    let features = index_scheduler.features()?;
+    let features = index_scheduler.features();
     let search_result = tokio::task::spawn_blocking(move || {
         perform_facet_search(&index, search_query, facet_query, facet_name, features)
     })

meilisearch/src/routes/indexes/search.rs

@@ -157,7 +157,7 @@ pub async fn search_with_url_query(
     let mut aggregate = SearchAggregator::from_query(&query, &req);
 
     let index = index_scheduler.index(&index_uid)?;
-    let features = index_scheduler.features()?;
+    let features = index_scheduler.features();
     let search_result =
         tokio::task::spawn_blocking(move || perform_search(&index, query, features)).await?;
     if let Ok(ref search_result) = search_result {
@@ -192,7 +192,7 @@ pub async fn search_with_post(
 
     let index = index_scheduler.index(&index_uid)?;
 
-    let features = index_scheduler.features()?;
+    let features = index_scheduler.features();
     let search_result =
         tokio::task::spawn_blocking(move || perform_search(&index, query, features)).await?;
     if let Ok(ref search_result) = search_result {

meilisearch/src/routes/metrics.rs

@@ -19,7 +19,7 @@ pub async fn get_metrics(
     index_scheduler: GuardedData<ActionPolicy<{ actions::METRICS_GET }>, Data<IndexScheduler>>,
     auth_controller: Data<AuthController>,
 ) -> Result<HttpResponse, ResponseError> {
-    index_scheduler.features()?.check_metrics()?;
+    index_scheduler.features().check_metrics()?;
     let auth_filters = index_scheduler.filters();
     if !auth_filters.all_indexes_authorized() {
         let mut error = ResponseError::from(AuthenticationError::InvalidToken);

meilisearch/src/routes/multi_search.rs

@@ -41,7 +41,7 @@ pub async fn multi_search_with_post(
     let queries = params.into_inner().queries;
 
     let mut multi_aggregate = MultiSearchAggregator::from_queries(&queries, &req);
-    let features = index_scheduler.features()?;
+    let features = index_scheduler.features();
 
     // Explicitly expect a `(ResponseError, usize)` for the error type rather than `ResponseError` only,
     // so that `?` doesn't work if it doesn't use `with_index`, ensuring that it is not forgotten in case of code