Skip to content

Commit 3578699

Browse files
committed
Simplify DependencyGroup::GetState() implementation
The new implementation just counts reachable and available parents and determines the overall result by comparing numbers, see inline comments for more information. This also fixes an issue in the previous implementation: if it didn't return early from the loop, it would just return the state of the last parent considered which may not actually represent the group state accurately.
1 parent e46b25d commit 3578699

File tree

1 file changed

+27
-17
lines changed

1 file changed

+27
-17
lines changed

lib/icinga/dependency-group.cpp

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -335,26 +335,36 @@ DependencyGroup::State DependencyGroup::GetState(DependencyType dt, int rstack)
335335
members = m_Members;
336336
}
337337

338-
State state{false /* Reachable */, false /* OK */};
339-
for (auto& [_, children] : members) {
340-
for (auto& [checkable, dependency] : children) {
341-
state.Reachable = dependency->GetParent()->IsReachable(dt, rstack);
342-
if (!state.Reachable && !IsRedundancyGroup()) {
343-
return state;
344-
}
338+
size_t reachable = 0, available = 0;
345339

346-
if (state.Reachable) {
347-
state.OK = dependency->IsAvailable(dt);
348-
// If this is a redundancy group, and we have found one functional path, that's enough and we can return.
349-
// Likewise, if this is a non-redundant dependency group, and we have found one non-functional path,
350-
// we have to mark the group as failed and return.
351-
if (state.OK == IsRedundancyGroup()) { // OK && IsRedundancyGroup() || !OK && !IsRedundancyGroup()
352-
return state;
353-
}
340+
for (auto& [_, dependencies] : members) {
341+
ASSERT(!dependencies.empty());
342+
343+
// Dependencies are grouped by parent and all config attributes affecting their availability. Hence, only
344+
// one dependency from the map entry has to be considered, all others will share the same state.
345+
auto& [child, dependency] = *dependencies.begin();
346+
347+
if (dependency->GetParent()->IsReachable(dt, rstack)) {
348+
reachable++;
349+
350+
// Only reachable parents are considered for availability. If they are unreachable and checks are disabled,
351+
// they could be incorrectly treated as available otherwise.
352+
if (dependency->IsAvailable(dt)) {
353+
available++;
354354
}
355-
break; // Move on to the next batch of group members (next composite key).
356355
}
357356
}
358357

359-
return state;
358+
if (IsRedundancyGroup()) {
359+
// The state of a redundancy group is determined by the best state of any parent. If any parent ist reachable,
360+
// the redundancy group is reachable, analogously for availability. Note that an unreachable group cannot be
361+
// available as reachable = 0 implies available = 0.
362+
return {reachable > 0, available > 0};
363+
} else {
364+
// For dependencies without a redundancy group, members.size() will be 1 in almost all cases. It will only
365+
// contain more elements if there duplicate dependency config objects between two checkables. In this case,
366+
// all of them have to reachable or available as they don't provide redundancy. Note that unreachable implies
367+
// unavailable here as well as only reachable parents count towards the number of available parents.
368+
return {reachable == members.size(), available == members.size()};
369+
}
360370
}

0 commit comments

Comments
 (0)