Evaluate dependency group state only for a specific child

Previously the dependency state was evaluated by picking the first
dependency object from the batched members. However, since the
dependency `disable_{checks,notifications` attributes aren't taken into
account when batching the members, the evaluated state may yield a wrong
result for some Checkables due to some random dependency from other
Checkable of that group that has the `disable_{checks,notifications`
attrs set. This commit forces the callers to always provide the child
Checkable the state is evaluated for and picks only the dependency
objects of that child Checkable.
This commit is contained in:
Yonas Habteab 2025-02-14 13:11:24 +01:00
parent ce1ed8556c
commit 7fbb8f7452
6 changed files with 20 additions and 25 deletions

View File

@ -206,7 +206,7 @@ bool Checkable::IsReachable(DependencyType dt, int rstack) const
}
for (auto& dependencyGroup : GetDependencyGroups()) {
if (auto state(dependencyGroup->GetState(dt, rstack + 1)); !state.Reachable || !state.OK) {
if (auto state(dependencyGroup->GetState(this, dt, rstack + 1)); !state.Reachable || !state.OK) {
Log(LogDebug, "Checkable")
<< "Dependency group '" << dependencyGroup->GetRedundancyGroupName() << "' have failed for checkable '"
<< GetName() << "': Marking as unreachable.";

View File

@ -311,29 +311,23 @@ String DependencyGroup::GetCompositeKey()
* a dependency group may still be marked as failed even when it has reachable parent Checkables, but an unreachable
* group has always a failed state.
*
* @param child The child Checkable to evaluate the state for.
* @param dt The dependency type to evaluate the state for, defaults to DependencyState.
* @param rstack The recursion stack level to prevent infinite recursion, defaults to 0.
*
* @return - Returns the state of the current dependency group.
*/
DependencyGroup::State DependencyGroup::GetState(DependencyType dt, int rstack) const
DependencyGroup::State DependencyGroup::GetState(const Checkable* child, DependencyType dt, int rstack) const
{
MembersMap members;
{
// We don't want to hold the mutex lock for the entire evaluation, thus we just need to operate on a copy.
std::lock_guard lock(m_Mutex);
members = m_Members;
}
auto dependencies(GetDependenciesForChild(child));
size_t reachable = 0, available = 0;
for (auto& [_, dependencies] : members) {
ASSERT(!dependencies.empty());
// Dependencies are grouped by parent and all config attributes affecting their availability. Hence, only
// one dependency from the map entry has to be considered, all others will share the same state.
if (auto& [_, dependency] = *dependencies.begin(); dependency->GetParent()->IsReachable(dt, rstack)) {
for (const auto& dependency : dependencies) {
if (dependency->GetParent()->IsReachable(dt, rstack)) {
reachable++;
// Only reachable parents are considered for availability. If they are unreachable and checks are disabled,
// they could be incorrectly treated as available otherwise.
// Only reachable parents are considered for availability. If they are unreachable and checks are
// disabled, they could be incorrectly treated as available otherwise.
if (dependency->IsAvailable(dt)) {
available++;
}
@ -346,10 +340,10 @@ DependencyGroup::State DependencyGroup::GetState(DependencyType dt, int rstack)
// available as reachable = 0 implies available = 0.
return {reachable > 0, available > 0};
} else {
// For dependencies without a redundancy group, members.size() will be 1 in almost all cases. It will only
// For dependencies without a redundancy group, dependencies.size() will be 1 in almost all cases. It will only
// contain more elements if there are duplicate dependency config objects between two checkables. In this case,
// all of them have to be reachable or available as they don't provide redundancy. Note that unreachable implies
// unavailable here as well as only reachable parents count towards the number of available parents.
return {reachable >= members.size(), available == members.size()};
return {reachable == dependencies.size(), available == dependencies.size()};
}
}

View File

@ -170,7 +170,7 @@ public:
bool OK; // Whether the dependency group is reachable and OK.
};
State GetState(DependencyType dt = DependencyState, int rstack = 0) const;
State GetState(const Checkable* child, DependencyType dt = DependencyState, int rstack = 0) const;
static boost::signals2::signal<void(const Checkable::Ptr&, const DependencyGroup::Ptr&)> OnChildRegistered;
static boost::signals2::signal<void(const DependencyGroup::Ptr&, const std::vector<Dependency::Ptr>&, bool)> OnChildRemoved;

View File

@ -1210,7 +1210,7 @@ void IcingaDB::InsertCheckableDependencies(
// to the DependencyEdgeState HMSETs. The latter is shared by all child Checkables of the current
// redundancy group, and since they all depend on the redundancy group, the state of that group is
// basically the state of the dependency edges between the children and the redundancy group.
auto stateAttrs(SerializeRedundancyGroupState(dependencyGroup));
auto stateAttrs(SerializeRedundancyGroupState(checkable, dependencyGroup));
AddDataToHmSets(hMSets, RedisKey::RedundancyGroupState, redundancyGroupId, stateAttrs);
AddDataToHmSets(hMSets, RedisKey::DependencyEdgeState, redundancyGroupId, Dictionary::Ptr(new Dictionary{
{"id", redundancyGroupId},
@ -1416,7 +1416,7 @@ void IcingaDB::UpdateDependenciesState(const Checkable::Ptr& checkable, const De
}
if (isRedundancyGroup) {
Dictionary::Ptr stateAttrs(SerializeRedundancyGroupState(dependencyGroup));
Dictionary::Ptr stateAttrs(SerializeRedundancyGroupState(checkable, dependencyGroup));
Dictionary::Ptr sharedGroupState(stateAttrs->ShallowClone());
sharedGroupState->Remove("redundancy_group_id");

View File

@ -201,13 +201,14 @@ Dictionary::Ptr IcingaDB::SerializeDependencyEdgeState(const DependencyGroup::Pt
/**
* Serialize the provided redundancy group state attributes.
*
* @param child The child checkable object to serialize the state for.
* @param redundancyGroup The redundancy group object to serialize the state for.
*
* @return A dictionary with the serialized redundancy group state.
*/
Dictionary::Ptr IcingaDB::SerializeRedundancyGroupState(const DependencyGroup::Ptr& redundancyGroup)
Dictionary::Ptr IcingaDB::SerializeRedundancyGroupState(const Checkable::Ptr& child, const DependencyGroup::Ptr& redundancyGroup)
{
auto state(redundancyGroup->GetState());
auto state(redundancyGroup->GetState(child.get()));
return new Dictionary{
{"id", redundancyGroup->GetIcingaDBIdentifier()},
{"environment_id", m_EnvironmentId},

View File

@ -175,7 +175,7 @@ private:
static const char* GetNotificationTypeByEnum(NotificationType type);
static Dictionary::Ptr SerializeVars(const Dictionary::Ptr& vars);
static Dictionary::Ptr SerializeDependencyEdgeState(const DependencyGroup::Ptr& dependencyGroup, const Dependency::Ptr& dep);
static Dictionary::Ptr SerializeRedundancyGroupState(const DependencyGroup::Ptr& redundancyGroup);
static Dictionary::Ptr SerializeRedundancyGroupState(const Checkable::Ptr& child, const DependencyGroup::Ptr& redundancyGroup);
static String HashValue(const Value& value);
static String HashValue(const Value& value, const std::set<String>& propertiesBlacklist, bool propertiesWhitelist = false);