Repository: meilisearch/meilisearch
File: index-scheduler/src/features.rs (Rust)

Code before:
    pub fn new(env: &Env, instance_features: InstanceTogglableFeatures) -> Result<Self> {
        let mut wtxn = env.write_txn()?;
        let runtime_features_db = env.create_database(&mut wtxn, Some(EXPERIMENTAL_FEATURES))?;
        wtxn.commit()?;

        let txn = env.read_txn()?;
        let persisted_features: RuntimeTogglableFeatures =
            runtime_features_db.get(&txn, EXPERIMENTAL_FEATURES)?.unwrap_or_default();
        let runtime = Arc::new(RwLock::new(RuntimeTogglableFeatures {
            metrics: instance_features.metrics || persisted_features.metrics,
            ..persisted_features
        }));

        Ok(Self { persisted: runtime_features_db, runtime })
    }

    pub fn put_runtime_features(
        &self,
        mut wtxn: RwTxn,
        features: RuntimeTogglableFeatures,
    ) -> Result<()> {
        self.persisted.put(&mut wtxn, EXPERIMENTAL_FEATURES, &features)?;
        wtxn.commit()?;

        let mut toggled_features = self.runtime.write().map_err(|_| RuntimeFeatureToggleError)?;
        *toggled_features = features;
        Ok(())
    }

    fn runtime_features(&self) -> Result<RuntimeTogglableFeatures> {
        Ok(*self.runtime.read().map_err(|_| RuntimeFeatureToggleError)?)
    }

    pub fn features(&self) -> Result<RoFeatures> {
        RoFeatures::new(self)
    }
}


Review comment (bug):
It is technically possible to end up in a weird state here where the runtime representation is different from what's persisted (your db write succeeds but your `RwLock` write fails). I did not think this would cause major problems due to the ability to retry the `put_runtime_features` that would rectify this discrepancy but I can handle this differently if you want!

Code after:
        let mut wtxn = env.write_txn()?;
        let runtime_features_db = env.create_database(&mut wtxn, Some(EXPERIMENTAL_FEATURES))?;
        wtxn.commit()?;

        let txn = env.read_txn()?;
        let persisted_features: RuntimeTogglableFeatures =
            runtime_features_db.get(&txn, EXPERIMENTAL_FEATURES)?.unwrap_or_default();
        let runtime = Arc::new(RwLock::new(RuntimeTogglableFeatures {
            metrics: instance_features.metrics || persisted_features.metrics,
            ..persisted_features
        }));

        Ok(Self { persisted: runtime_features_db, runtime })
    }

    pub fn put_runtime_features(
        &self,
        mut wtxn: RwTxn,
        features: RuntimeTogglableFeatures,
    ) -> Result<()> {
        self.persisted.put(&mut wtxn, EXPERIMENTAL_FEATURES, &features)?;
        wtxn.commit()?;

        // 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) -> 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) -> RoFeatures {
        RoFeatures::new(self)
    }
}