From 1a739fc3bfd9627784e35d793452cd2d0da4513d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jan=20Kundr=C3=A1t?= Date: Tue, 11 Mar 2014 11:00:22 +0100 Subject: [PATCH] thresholds: Fix calculation of hysteresis The old code would never emit a notification when the value changed between the WARNING and FAILURE; the reason was that each branch in the switch statement only checked the "old" thresholds valid for the previously encountered state. That is wrong and pretty dangerous, as there will be no notification when a values progresses slowly from OK to WARN to FAIL. Fixes #578. --- src/threshold.c | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/threshold.c b/src/threshold.c index a4449741..5215c9ca 100644 --- a/src/threshold.c +++ b/src/threshold.c @@ -749,23 +749,40 @@ static int ut_check_one_data_source (const data_set_t *ds, /* XXX: This is an experimental code, not optimized, not fast, not reliable, * and probably, do not work as you expect. Enjoy! :D */ - if ( (th->hysteresis > 0) && ((prev_state = uc_get_state(ds,vl)) != STATE_OKAY) ) + if (th->hysteresis > 0) { - switch(prev_state) + prev_state = uc_get_state(ds,vl); + /* The purpose of hysteresis is elliminating flapping state when the value + * oscilates around the thresholds. In other words, what is important is + * the previous state; if the new value would trigger a transition, make + * sure that we artificially widen the range which is considered to apply + * for the previous state, and only trigger the notification if the value + * is outside of this expanded range. + * + * There is no hysteresis for the OKAY state. + * */ + gauge_t hysteresis_for_warning = 0, hysteresis_for_failure = 0; + switch (prev_state) { case STATE_ERROR: - if ( (!isnan (th->failure_min) && ((th->failure_min + th->hysteresis) < values[ds_index])) || - (!isnan (th->failure_max) && ((th->failure_max - th->hysteresis) > values[ds_index])) ) - return (STATE_OKAY); - else - is_failure++; + hysteresis_for_failure = th->hysteresis; + break; case STATE_WARNING: - if ( (!isnan (th->warning_min) && ((th->warning_min + th->hysteresis) < values[ds_index])) || - (!isnan (th->warning_max) && ((th->warning_max - th->hysteresis) > values[ds_index])) ) - return (STATE_OKAY); - else - is_warning++; - } + hysteresis_for_warning = th->hysteresis; + break; + case STATE_OKAY: + /* do nothing -- the hysteresis only applies to the non-normal states */ + break; + } + + if ((!isnan (th->failure_min) && (th->failure_min + hysteresis_for_failure > values[ds_index])) + || (!isnan (th->failure_max) && (th->failure_max - hysteresis_for_failure < values[ds_index]))) + is_failure++; + + if ((!isnan (th->warning_min) && (th->warning_min + hysteresis_for_warning > values[ds_index])) + || (!isnan (th->warning_max) && (th->warning_max - hysteresis_for_warning < values[ds_index]))) + is_warning++; + } else { /* no hysteresis */ if ((!isnan (th->failure_min) && (th->failure_min > values[ds_index])) -- 2.30.2