Checkable: Don't recalculate `next_check` while processing remotely genrated check

Currently, when processing a `CheckResult`, it will first trigger an
`OnNextCheckChanged` event, which is sent to all connected endpoints.
Then, when `Checkable::ProcessCheckResult()` returns, an `OnCheckResult`
event is fired, which is of course also sent to all connected endpoints.

Next, the other endpoints receive the `event::SetNextCheck` cluster
event followed by `event::CheckResult`and invoke
`checkable#SetNextCheck()` and `Checkable#CheckResult()` with the newly
received check. So they also try to recalculate the next check
themselves and invalidate the previously received next check timestamp
from the source endpoint. Since each endpoint randomly initialises its
own scheduling offset, the recalculated next check will always differ by
a split second/millisecond on each of them. As a consequence, two Icinga
DB HA instances will generate two different checksums for the same state
and causes the state histories to be fully resynchronised after a
takeover/Icinga 2 reload.
This commit is contained in:
Yonas Habteab 2024-02-29 10:47:55 +01:00 committed by Alexander A. Klimov
parent adc350da08
commit f66f99516b
1 changed files with 20 additions and 13 deletions

View File

@ -358,21 +358,28 @@ void Checkable::ProcessCheckResult(const CheckResult::Ptr& cr, const MessageOrig
bool is_flapping = IsFlapping();
if (cr->GetActive()) {
UpdateNextCheck(origin);
} else {
/* Reschedule the next check for external passive check results. The side effect of
* this is that for as long as we receive results for a service we
* won't execute any active checks. */
double offset;
double ttl = cr->GetTtl();
// Don't recompute the next check when the current check isn't generated by this endpoint. When the check is
// remotely generated we should've already received the "SetNextCheck" event before the "event::CheckResult"
// cluster event. Otherwise, the next check received before this check will be invalidated and cause the Checkable
// "next_check/next_update" in a HA setup to always be different from the other endpoint as the "m_SchedulingOffset"
// is randomly initialised on each node.
if (!origin) {
if (cr->GetActive()) {
UpdateNextCheck();
} else {
/* Reschedule the next check for external passive check results. The side effect of
* this is that for as long as we receive results for a service we
* won't execute any active checks. */
double offset;
double ttl = cr->GetTtl();
if (ttl > 0)
offset = ttl;
else
offset = GetCheckInterval();
if (ttl > 0)
offset = ttl;
else
offset = GetCheckInterval();
SetNextCheck(Utility::GetTime() + offset, false, origin);
SetNextCheck(Utility::GetTime() + offset);
}
}
olock.Unlock();