Disabling the lifo slot by default, and variable shard load (#5898)

Co-authored-by: Paul Masurel <paul@quickwit.io>

Author: fulmicoton-dd

quickwit/quickwit-common/src/lib.rs

@@ -84,43 +84,42 @@ pub fn split_file(split_id: impl Display) -> String {
     format!("{split_id}.split")
 }
 
-pub fn get_from_env<T: FromStr + Debug>(key: &str, default_value: T) -> T {
-    if let Ok(value_str) = std::env::var(key) {
-        if let Ok(value) = T::from_str(&value_str) {
-            info!(value=?value, "using environment variable `{key}` value");
-            return value;
-        } else {
-            error!(value=%value_str, "failed to parse environment variable `{key}` value");
-        }
-    }
-    info!(value=?default_value, "using environment variable `{key}` default value");
-    default_value
+fn get_from_env_opt_aux<T: Debug>(
+    key: &str,
+    parse_fn: impl FnOnce(&str) -> Option<T>,
+) -> Option<T> {
+    let value_str = std::env::var(key).ok()?;
+    let Some(value) = parse_fn(&value_str) else {
+        error!(value=%value_str, "failed to parse environment variable `{key}` value");
+        return None;
+    };
+    info!(value=?value, "using environment variable `{key}` value");
+    Some(value)
 }
 
-pub fn get_bool_from_env(key: &str, default_value: bool) -> bool {
-    if let Ok(value_str) = std::env::var(key) {
-        if let Some(value) = parse_bool_lenient(&value_str) {
-            info!(value=%value, "using environment variable `{key}` value");
-            return value;
-        } else {
-            error!(value=%value_str, "failed to parse environment variable `{key}` value");
-        }
+pub fn get_from_env<T: FromStr + Debug>(key: &str, default_value: T) -> T {
+    if let Some(value) = get_from_env_opt(key) {
+        value
+    } else {
+        info!(default_value=?default_value, "using environment variable `{key}` default value");
+        default_value
     }
-    info!(value=?default_value, "using environment variable `{key}` default value");
-    default_value
 }
 
 pub fn get_from_env_opt<T: FromStr + Debug>(key: &str) -> Option<T> {
-    let Some(value_str) = std::env::var(key).ok() else {
-        info!("environment variable `{key}` is not set");
-        return None;
-    };
-    if let Ok(value) = T::from_str(&value_str) {
-        info!(value=?value, "using environment variable `{key}` value");
-        Some(value)
+    get_from_env_opt_aux(key, |val_str| val_str.parse().ok())
+}
+
+pub fn get_bool_from_env_opt(key: &str) -> Option<bool> {
+    get_from_env_opt_aux(key, parse_bool_lenient)
+}
+
+pub fn get_bool_from_env(key: &str, default_value: bool) -> bool {
+    if let Some(flag_value) = get_bool_from_env_opt(key) {
+        flag_value
     } else {
-        error!(value=%value_str, "failed to parse environment variable `{key}` value");
-        None
+        info!(default_value=%default_value, "using environment variable `{key}` default value");
+        default_value
     }
 }
 

quickwit/quickwit-common/src/runtimes.rs

@@ -97,7 +97,7 @@ impl Default for RuntimesConfig {
 fn start_runtimes(config: RuntimesConfig) -> HashMap<RuntimeType, Runtime> {
     let mut runtimes = HashMap::with_capacity(2);
 
-    let disable_lifo_slot = crate::get_bool_from_env("QW_DISABLE_TOKIO_LIFO_SLOT", false);
+    let disable_lifo_slot = crate::get_bool_from_env("QW_DISABLE_TOKIO_LIFO_SLOT", true);
 
     let mut blocking_runtime_builder = tokio::runtime::Builder::new_multi_thread();
     if disable_lifo_slot {

quickwit/quickwit-control-plane/src/indexing_scheduler/mod.rs

@@ -42,6 +42,8 @@ use crate::metrics::ShardLocalityMetrics;
 use crate::model::{ControlPlaneModel, ShardEntry, ShardLocations};
 use crate::{IndexerNodeInfo, IndexerPool};
 
+const DEFAULT_ENABLE_VARIABLE_SHARD_LOAD: bool = false;
+
 pub(crate) const MIN_DURATION_BETWEEN_SCHEDULING: Duration =
     if cfg!(any(test, feature = "testsuite")) {
         Duration::from_millis(50)
@@ -121,7 +123,30 @@ impl fmt::Debug for IndexingScheduler {
 fn enable_variable_shard_load() -> bool {
     static IS_SHARD_LOAD_CP_ENABLED: OnceCell<bool> = OnceCell::new();
     *IS_SHARD_LOAD_CP_ENABLED.get_or_init(|| {
-        !quickwit_common::get_bool_from_env("QW_DISABLE_VARIABLE_SHARD_LOAD", false)
+        if let Some(enable_flag) =
+            quickwit_common::get_bool_from_env_opt("QW_ENABLE_VARIABLE_SHARD_LOAD")
+        {
+            return enable_flag;
+        }
+        // For backward compatibility, if QW_DISABLE_VARIABLE_SHARD_LOAD is set, we accept this
+        // value too.
+        if let Some(disable_flag) =
+            quickwit_common::get_bool_from_env_opt("QW_DISABLE_VARIABLE_SHARD_LOAD")
+        {
+            warn!(
+                disable = disable_flag,
+                "QW_DISABLE_VARIABLE_SHARD_LOAD is deprecated. Please use \
+                 QW_ENABLE_VARIABLE_SHARD_LOAD instead. We will use your setting in this version, \
+                 but will likely ignore it in future versions."
+            );
+            return !disable_flag;
+        }
+        // Defaulting to false
+        info!(
+            "QW_ENABLE_VARIABLE_SHARD_LOAD not set, defaulting to {}",
+            DEFAULT_ENABLE_VARIABLE_SHARD_LOAD
+        );
+        DEFAULT_ENABLE_VARIABLE_SHARD_LOAD
     })
 }