Fix QoS division by 0

While the count_above_threshold does have a check for 0, it doesn't do
what we want; we want it to return an infinite value somehow, but it
instead returns the dividend. This patch checks for this case and
returns the correct value manually.

The reason why it is important to return infinite value in this case is
that this division is supposed to always increase the threshold value.
Usually it does that correctly, since remaining_throughput has been
decreased by only those values that were under the average value, and
count_above_threshold has been decreased by count of those values.
Therefore, the result of the division is the average of all the values
that weren't lower than the previous average, and this average obviously
can't be lower than the previous average. However, if all the values are
under the average, the new average doesn't exist. It's morally infinite,
but the division instead returns the remaining_throughput, which is
obviously much lower than the original throughput, and often lower than
the threshold value. For example, if throughput is 20 and we have one
log source having written 18 logs, the result will be 2, which is wrong.

Change-Id: Id6d085707aeabc4032f931869c213f01db2c927a

diff --git a/src/logger/qos_distributions.c b/src/logger/qos_distributions.c
index c7faf6e..2fe499f 100644 (file)
--- a/src/logger/qos_distributions.c
+++ b/src/logger/qos_distributions.c
@@ -3,9 +3,12 @@
 #include <assert.h>
 #include <stdlib.h>
 
-static inline int divide_rounding_upwards(int dividend, int divisor)
+static inline int divide_rounding_upwards_or_inf(int dividend, int divisor)
 {
-       return (dividend + (divisor - 1)) / (divisor ?: 1);
+       if (divisor == 0)
+               return -1; // QoS interprets -1 as infinite
+
+       return (dividend + (divisor - 1)) / divisor;
 }
 
 static inline int sort_by_count(const void *vlhs, const void *vrhs)
@@ -26,7 +29,7 @@ void qos_distribution_equal(struct qos_module *qos, struct metrics_pid_aggr_info
 {
        /* round upwards so that it's never 0, to give all clients a chance
         * to log at least something (so a developer can tell it's alive) */
-       const int equal_limit_for_everybody = divide_rounding_upwards(qos->max_throughput, count);
+       const int equal_limit_for_everybody = divide_rounding_upwards_or_inf(qos->max_throughput, count);
 
        for (int i = 0; i < count; ++i)
                infos[i].count = equal_limit_for_everybody;
@@ -51,7 +54,7 @@ void qos_distribution_equal_dual(struct qos_module *qos, struct metrics_pid_aggr
                -- count_above_threshold;
        }
 
-       const int higher_threshold = divide_rounding_upwards(remaining_throughput, count_above_threshold);
+       const int higher_threshold = divide_rounding_upwards_or_inf(remaining_throughput, count_above_threshold);
        for (int i = 0; i < count; ++i)
                if (infos[i].count >= equal_threshold)
                        infos[i].count = higher_threshold;
@@ -84,7 +87,7 @@ void qos_distribution_equal_multi(struct qos_module *qos, struct metrics_pid_agg
                        ++ below_current_threshold;
                }
                prev_threshold = threshold;
-               threshold = divide_rounding_upwards(remaining_throughput, count_above_threshold);
+               threshold = divide_rounding_upwards_or_inf(remaining_throughput, count_above_threshold);
        } while (below_current_threshold > 0);
 
        for (int i = 0; i < count; ++i)