From 5334cb98eabe87c54f69823a6b5408c3a1745557 Mon Sep 17 00:00:00 2001
From: Theo Butler <theodusbutler@gmail.com>
Date: Wed, 29 Nov 2023 09:21:09 -0500
Subject: [PATCH] fix(budget): reduce windup

---
 graph-gateway/src/budgets.rs | 59 +++++-------------------------------
 1 file changed, 7 insertions(+), 52 deletions(-)

diff --git a/graph-gateway/src/budgets.rs b/graph-gateway/src/budgets.rs
index e0a52a9b9..d352a98d7 100644
--- a/graph-gateway/src/budgets.rs
+++ b/graph-gateway/src/budgets.rs
@@ -97,13 +97,11 @@ struct Controller {
 
 impl Controller {
     fn new(target_query_fees: USD) -> Self {
-        let mut error_history = FastDecayBuffer::default();
-        *error_history.current_mut() = target_query_fees.0.into();
         Self {
             target_query_fees,
             recent_fees: USD(UDecimal18::from(0)),
             recent_query_count: 0,
-            error_history,
+            error_history: FastDecayBuffer::default(),
         }
     }
 
@@ -115,6 +113,7 @@ impl Controller {
     fn control_variable(&mut self) -> UDecimal18 {
         // See the following link if you're unfamiliar with PID controllers:
         // https://en.wikipedia.org/wiki/Proportional%E2%80%93integral%E2%80%93derivative_controller
+        let target = f64::from(self.target_query_fees.0);
         let process_variable =
             f64::from(self.recent_fees.0) / self.recent_query_count.max(1) as f64;
         METRICS.avg_query_fees.set(process_variable);
@@ -122,56 +121,12 @@ impl Controller {
         self.recent_fees = USD(UDecimal18::from(0));
         self.recent_query_count = 0;
         self.error_history.decay();
-        let error = f64::from(self.target_query_fees.0) - process_variable;
-        *self.error_history.current_mut() = error;
+        let error = target - process_variable;
+        *self.error_history.current_mut() += error;
 
         let i: f64 = self.error_history.frames().iter().sum();
-        let k_i = 1.2;
-        let correction = UDecimal18::try_from(i * k_i).unwrap_or_default();
-        self.target_query_fees.0 + correction
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use indexer_selection::test_utils::assert_within;
-
-    use super::*;
-
-    #[test]
-    fn controller() {
-        fn test_controller(
-            controller: &mut Controller,
-            process_variable_multiplier: f64,
-            tolerance: f64,
-        ) {
-            let setpoint: f64 = controller.target_query_fees.0.into();
-            let mut process_variable = 0.0;
-            for i in 0..20 {
-                let control_variable: f64 = controller.control_variable().into();
-                process_variable = control_variable * process_variable_multiplier;
-                println!(
-                    "{i:02} SP={setpoint:.6}, PV={:.8}, CV={:.8}",
-                    process_variable, control_variable,
-                );
-                controller.add_queries(USD(UDecimal18::try_from(process_variable).unwrap()), 1);
-            }
-            assert_within(process_variable, setpoint, tolerance);
-        }
-
-        for setpoint in [10e-6, 20e-6, 50e-6] {
-            let setpoint = USD(UDecimal18::try_from(setpoint).unwrap());
-            let mut controller = Controller::new(setpoint);
-            test_controller(&mut controller, 0.2, 1e-6);
-            let mut controller = Controller::new(setpoint);
-            test_controller(&mut controller, 0.6, 1e-6);
-            let mut controller = Controller::new(setpoint);
-            test_controller(&mut controller, 0.8, 1e-6);
-
-            let mut controller = Controller::new(setpoint);
-            test_controller(&mut controller, 0.2, 1e-6);
-            test_controller(&mut controller, 0.6, 1e-6);
-            test_controller(&mut controller, 0.7, 1e-6);
-        }
+        let k_i = 0.003;
+        let cv = i * k_i;
+        UDecimal18::try_from(target + cv).unwrap_or(self.target_query_fees.0)
     }
 }