Fix bug in EventRateMeter

It was relying on ExponentiallySmoothedCounter::get() which is designed for specific 1 second time interval between points. Now sum of weights is computed separatly in `duration` field, giving very accurate measurements independent of interval.

src/Common/EventRateMeter.h

@@ -4,8 +4,6 @@
 
 #include <Common/ExponentiallySmoothedCounter.h>
 
-#include <numbers>
-
 
 namespace DB
 {
@@ -14,10 +12,10 @@ namespace DB
 class EventRateMeter
 {
 public:
-    explicit EventRateMeter(double now, double period_, double step_ = 0.0)
+    explicit EventRateMeter(double now, double period_, size_t heating_ = 0)
         : period(period_)
-        , step(step_)
-        , half_decay_time(period * std::numbers::ln2) // for `ExponentiallySmoothedAverage::sumWeights()` to be equal to `1/period`
+        , max_interval(period * 10)
+        , heating(heating_)
     {
         reset(now);
     }
@@ -30,25 +28,11 @@ public:
     {
         // Remove data for initial heating stage that can present at the beginning of a query.
         // Otherwise it leads to wrong gradual increase of average value, turning algorithm into not very reactive.
-        if (count != 0.0 && ++data_points < 5)
-        {
-            start = events.time;
-            events = ExponentiallySmoothedAverage();
-        }
+        if (count != 0.0 && data_points++ <= heating)
+            reset(events.time, data_points);
 
-        if (now - period <= start) // precise counting mode
-            events = ExponentiallySmoothedAverage(events.value + count, now);
-        else // exponential smoothing mode
-        {
-            // Adding events too often lead to low precision due to smoothing too often, so we buffer new events and add them in steps
-            step_count += count;
-            if (step_start + step <= now)
-            {
-                events.add(step_count, now, half_decay_time);
-                step_start = now;
-                step_count = 0;
-            }
-        }
+        duration.add(std::min(max_interval, now - duration.time), now, period);
+        events.add(count, now, period);
     }
 
     /// Compute average event rate throughout `[now - period, now]` period.
@@ -59,29 +43,27 @@ public:
         add(now, 0);
         if (unlikely(now <= start))
             return 0;
-        if (now - period <= start) // precise counting mode
-            return events.value / (now - start);
-        else // exponential smoothing mode
-            return events.get(half_decay_time); // equals to `events.value / period`
+
+        // We do not use .get() because sum of weights will anyway be canceled out (optimization)
+        return events.value / duration.value;
     }
 
-    void reset(double now)
+    void reset(double now, size_t data_points_ = 0)
     {
         start = now;
-        step_start = now;
         events = ExponentiallySmoothedAverage();
-        data_points = 0;
+        duration = ExponentiallySmoothedAverage();
+        data_points = data_points_;
     }
 
 private:
     const double period;
-    const double step; // duration of a step
-    const double half_decay_time;
+    const double max_interval;
+    const size_t heating;
     double start; // Instant in past without events before it; when measurement started or reset
-    ExponentiallySmoothedAverage events; // Estimated number of events in the last `period`
+    ExponentiallySmoothedAverage duration; // Current duration of a period
+    ExponentiallySmoothedAverage events; // Estimated number of events in last `duration` seconds
     size_t data_points = 0;
-    double step_start; // start instant of the last step
-    double step_count = 0.0; // number of events accumulated since step start
 };
 
 }

src/Common/ProgressIndication.h

@@ -91,7 +91,7 @@ private:
 
     bool write_progress_on_update = false;
 
-    EventRateMeter cpu_usage_meter{static_cast<double>(clock_gettime_ns()), 2'000'000'000 /*ns*/}; // average cpu utilization last 2 second
+    EventRateMeter cpu_usage_meter{static_cast<double>(clock_gettime_ns()), 2'000'000'000 /*ns*/, 4}; // average cpu utilization last 2 second, skip first 4 points
     HostToTimesMap hosts_data;
     /// In case of all of the above:
     /// - clickhouse-local

src/Common/tests/gtest_event_rate_meter.cpp

@@ -0,0 +1,68 @@
+#include <gtest/gtest.h>
+
+#include <Common/EventRateMeter.h>
+
+#include <cmath>
+
+
+TEST(EventRateMeter, ExponentiallySmoothedAverage)
+{
+    double target = 100.0;
+
+    // The test is only correct for timestep of 1 second because of
+    // how sum of weights is implemented inside `ExponentiallySmoothedAverage`
+    double time_step = 1.0;
+
+    for (double half_decay_time : { 0.1, 1.0, 10.0, 100.0})
+    {
+        DB::ExponentiallySmoothedAverage esa;
+
+        int steps = static_cast<int>(half_decay_time * 30 / time_step);
+        for (int i = 1; i <= steps; ++i)
+            esa.add(target * time_step, i * time_step, half_decay_time);
+        double measured = esa.get(half_decay_time);
+        ASSERT_LE(std::fabs(measured - target), 1e-5 * target);
+    }
+}
+
+TEST(EventRateMeter, ConstantRate)
+{
+    double target = 100.0;
+
+    for (double period : {0.1, 1.0, 10.0})
+    {
+        for (double time_step : {0.001, 0.01, 0.1, 1.0})
+        {
+            DB::EventRateMeter erm(0.0, period);
+
+            int steps = static_cast<int>(period * 30 / time_step);
+            for (int i = 1; i <= steps; ++i)
+                erm.add(i * time_step, target * time_step);
+            double measured = erm.rate(steps * time_step);
+            // std::cout << "T=" << period << " dt=" << time_step << " measured=" << measured << std::endl;
+            ASSERT_LE(std::fabs(measured - target), 1e-5 * target);
+        }
+    }
+}
+
+TEST(EventRateMeter, PreciseStart)
+{
+    double target = 100.0;
+
+    for (double period : {0.1, 1.0, 10.0})
+    {
+        for (double time_step : {0.001, 0.01, 0.1, 1.0})
+        {
+            DB::EventRateMeter erm(0.0, period);
+
+            int steps = static_cast<int>(period / time_step);
+            for (int i = 1; i <= steps; ++i)
+            {
+                erm.add(i * time_step, target * time_step);
+                double measured = erm.rate(i * time_step);
+                // std::cout << "T=" << period << " dt=" << time_step << " measured=" << measured << std::endl;
+                ASSERT_LE(std::fabs(measured - target), 1e-5 * target);
+            }
+        }
+    }
+}