Skip to content

Commit 92df9ef

Browse files
yhabteabyhabteab
authored
Merge pull request #10148 from Icinga/enhanced-sort-types-by-load-dependencies
Sort config types by their load dependencies once
2 parents 0fff415 + eb97676 commit 92df9ef

File tree

6 files changed

+224
-131
lines changed

6 files changed

+224
-131
lines changed

‎lib/base/initialize.hpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ enum class InitializePriority {
2323
RegisterBuiltinTypes,
2424
RegisterFunctions,
2525
RegisterTypes,
26+
SortTypes,
2627
EvaluateConfigFragments,
2728
Default,
2829
FreezeNamespaces,

‎lib/base/type.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
/* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
22

33
#include "base/type.hpp"
4+
#include "base/atomic.hpp"
5+
#include "base/configobject.hpp"
6+
#include "base/debug.hpp"
47
#include "base/scriptglobal.hpp"
58
#include "base/namespace.hpp"
69
#include "base/objectlock.hpp"
10+
#include <functional>
711

812
using namespace icinga;
913

@@ -32,6 +36,43 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() {
3236
Type::Register(type);
3337
}, InitializePriority::RegisterTypeType);
3438

39+
static std::vector<Type::Ptr> l_SortedByLoadDependencies;
40+
static Atomic l_SortingByLoadDependenciesDone (false);
41+
42+
INITIALIZE_ONCE_WITH_PRIORITY([] {
43+
std::unordered_set<Type*> visited;
44+
45+
std::function<void(Type*)> visit;
46+
// Please note that this callback does not detect any cyclic load dependencies,
47+
// instead, it relies on the "sort_by_load_after" unit test to fail.
48+
visit = ([&visit, &visited](Type* type) {
49+
if (visited.find(type) != visited.end()) {
50+
return;
51+
}
52+
visited.emplace(type);
53+
54+
for (auto dependency : type->GetLoadDependencies()) {
55+
visit(dependency);
56+
}
57+
58+
// We have managed to reach the final/top node in this dependency graph,
59+
// so let's place them in reverse order to their final place.
60+
l_SortedByLoadDependencies.emplace_back(type);
61+
});
62+
63+
// Sort the types by their load_after dependencies in a Depth-First search manner.
64+
for (const Type::Ptr& type : Type::GetAllTypes()) {
65+
// Note that only those types that are assignable to the dynamic ConfigObject type can have "load_after"
66+
// dependencies, otherwise they are just some Icinga 2 primitive types such as Number, String, etc. and
67+
// we need to ignore them.
68+
if (ConfigObject::TypeInstance->IsAssignableFrom(type)) {
69+
visit(type.get());
70+
}
71+
}
72+
73+
l_SortingByLoadDependenciesDone.store(true);
74+
}, InitializePriority::SortTypes);
75+
3576
String Type::ToString() const
3677
{
3778
return "type '" + GetName() + "'";
@@ -72,6 +113,12 @@ std::vector<Type::Ptr> Type::GetAllTypes()
72113
return types;
73114
}
74115

116+
const std::vector<Type::Ptr>& Type::GetConfigTypesSortedByLoadDependencies()
117+
{
118+
VERIFY(l_SortingByLoadDependenciesDone.load());
119+
return l_SortedByLoadDependencies;
120+
}
121+
75122
String Type::GetPluralName() const
76123
{
77124
String name = GetName();

‎lib/base/type.hpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,21 @@ class Type : public Object
8383
static Type::Ptr GetByName(const String& name);
8484
static std::vector<Type::Ptr> GetAllTypes();
8585

86+
/**
87+
* Returns a list of config types sorted by their "load_after" dependencies.
88+
*
89+
* All dependencies of a given type are listed at a lower index than that of the type itself. In other words,
90+
* if a `Service` type load depends on the `Host` and `ApiListener` types, the Host and ApiListener types are
91+
* guaranteed to appear first on the list. Nevertheless, the order of the Host and ApiListener types themselves
92+
* is arbitrary if the two types are not dependent.
93+
*
94+
* It should be noted that this method will fail fatally when used prior to the completion
95+
* of namespace initialization.
96+
*
97+
* @return std::vector<Type::Ptr>
98+
*/
99+
static const std::vector<Ptr>& GetConfigTypesSortedByLoadDependencies();
100+
86101
void SetField(int id, const Value& value, bool suppress_events = false, const Value& cookie = Empty) override;
87102
Value GetField(int id) const override;
88103

‎lib/config/configitem.cpp‎

Lines changed: 76 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -444,180 +444,131 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
444444
<< "Committing " << total << " new items.";
445445
#endif /* I2_DEBUG */
446446

447-
std::set<Type::Ptr> types;
448-
std::set<Type::Ptr> completed_types;
449447
int itemsCount {0};
450448

451-
for (const Type::Ptr& type : Type::GetAllTypes()) {
452-
if (ConfigObject::TypeInstance->IsAssignableFrom(type))
453-
types.insert(type);
454-
}
455-
456-
while (types.size() != completed_types.size()) {
457-
for (const Type::Ptr& type : types) {
458-
if (completed_types.find(type) != completed_types.end())
459-
continue;
449+
for (auto& type : Type::GetConfigTypesSortedByLoadDependencies()) {
450+
std::atomic<int> committed_items(0);
460451

461-
bool unresolved_dep = false;
452+
{
453+
auto items (itemsByType.find(type.get()));
462454

463-
/* skip this type (for now) if there are unresolved load dependencies */
464-
for (auto pLoadDep : type->GetLoadDependencies()) {
465-
if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) {
466-
unresolved_dep = true;
467-
break;
455+
if (items != itemsByType.end()) {
456+
for (const ItemPair& pair: items->second) {
457+
newItems.emplace_back(pair.first);
468458
}
469-
}
470-
471-
if (unresolved_dep)
472-
continue;
473459

474-
std::atomic<int> committed_items(0);
460+
upq.ParallelFor(items->second, [&committed_items](const ItemPair& ip) {
461+
const ConfigItem::Ptr& item = ip.first;
475462

476-
{
477-
auto items (itemsByType.find(type.get()));
463+
if (!item->Commit(ip.second)) {
464+
if (item->IsIgnoreOnError()) {
465+
item->Unregister();
466+
}
478467

479-
if (items != itemsByType.end()) {
480-
for (const ItemPair& pair: items->second) {
481-
newItems.emplace_back(pair.first);
468+
return;
482469
}
483470

484-
upq.ParallelFor(items->second, [&committed_items](const ItemPair& ip) {
485-
const ConfigItem::Ptr& item = ip.first;
471+
committed_items++;
472+
});
486473

487-
if (!item->Commit(ip.second)) {
488-
if (item->IsIgnoreOnError()) {
489-
item->Unregister();
490-
}
491-
492-
return;
493-
}
494-
495-
committed_items++;
496-
});
497-
498-
upq.Join();
499-
}
474+
upq.Join();
500475
}
476+
}
501477

502-
itemsCount += committed_items;
503-
504-
completed_types.insert(type);
478+
itemsCount += committed_items;
505479

506480
#ifdef I2_DEBUG
507-
if (committed_items > 0)
508-
Log(LogDebug, "configitem")
509-
<< "Committed " << committed_items << " items of type '" << type->GetName() << "'.";
481+
if (committed_items > 0)
482+
Log(LogDebug, "configitem")
483+
<< "Committed " << committed_items << " items of type '" << type->GetName() << "'.";
510484
#endif /* I2_DEBUG */
511485

512-
if (upq.HasExceptions())
513-
return false;
514-
}
486+
if (upq.HasExceptions())
487+
return false;
515488
}
516489

517490
#ifdef I2_DEBUG
518491
Log(LogDebug, "configitem")
519492
<< "Committed " << itemsCount << " items.";
520493
#endif /* I2_DEBUG */
521494

522-
completed_types.clear();
495+
for (auto& type : Type::GetConfigTypesSortedByLoadDependencies()) {
496+
std::atomic<int> notified_items(0);
523497

524-
while (types.size() != completed_types.size()) {
525-
for (const Type::Ptr& type : types) {
526-
if (completed_types.find(type) != completed_types.end())
527-
continue;
498+
{
499+
auto items (itemsByType.find(type.get()));
528500

529-
bool unresolved_dep = false;
501+
if (items != itemsByType.end()) {
502+
upq.ParallelFor(items->second, [&notified_items](const ItemPair& ip) {
503+
const ConfigItem::Ptr& item = ip.first;
530504

531-
/* skip this type (for now) if there are unresolved load dependencies */
532-
for (auto pLoadDep : type->GetLoadDependencies()) {
533-
if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) {
534-
unresolved_dep = true;
535-
break;
536-
}
537-
}
505+
if (!item->m_Object)
506+
return;
538507

539-
if (unresolved_dep)
540-
continue;
541-
542-
std::atomic<int> notified_items(0);
543-
544-
{
545-
auto items (itemsByType.find(type.get()));
546-
547-
if (items != itemsByType.end()) {
548-
upq.ParallelFor(items->second, [&notified_items](const ItemPair& ip) {
549-
const ConfigItem::Ptr& item = ip.first;
550-
551-
if (!item->m_Object)
552-
return;
508+
try {
509+
item->m_Object->OnAllConfigLoaded();
510+
notified_items++;
511+
} catch (const std::exception& ex) {
512+
if (!item->m_IgnoreOnError)
513+
throw;
553514

554-
try {
555-
item->m_Object->OnAllConfigLoaded();
556-
notified_items++;
557-
} catch (const std::exception& ex) {
558-
if (!item->m_IgnoreOnError)
559-
throw;
515+
Log(LogNotice, "ConfigObject")
516+
<< "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex);
560517

561-
Log(LogNotice, "ConfigObject")
562-
<< "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex);
518+
item->Unregister();
563519

564-
item->Unregister();
565-
566-
{
567-
std::unique_lock<std::mutex> lock(item->m_Mutex);
568-
item->m_IgnoredItems.push_back(item->m_DebugInfo.Path);
569-
}
520+
{
521+
std::unique_lock<std::mutex> lock(item->m_Mutex);
522+
item->m_IgnoredItems.push_back(item->m_DebugInfo.Path);
570523
}
571-
});
524+
}
525+
});
572526

573-
upq.Join();
574-
}
527+
upq.Join();
575528
}
576-
577-
completed_types.insert(type);
529+
}
578530

579531
#ifdef I2_DEBUG
580-
if (notified_items > 0)
581-
Log(LogDebug, "configitem")
582-
<< "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'.";
532+
if (notified_items > 0)
533+
Log(LogDebug, "configitem")
534+
<< "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'.";
583535
#endif /* I2_DEBUG */
584536

585-
if (upq.HasExceptions())
586-
return false;
537+
if (upq.HasExceptions())
538+
return false;
587539

588-
notified_items = 0;
589-
for (auto loadDep : type->GetLoadDependencies()) {
590-
auto items (itemsByType.find(loadDep));
540+
notified_items = 0;
541+
for (auto loadDep : type->GetLoadDependencies()) {
542+
auto items (itemsByType.find(loadDep));
591543

592-
if (items != itemsByType.end()) {
593-
upq.ParallelFor(items->second, [&type, &notified_items](const ItemPair& ip) {
594-
const ConfigItem::Ptr& item = ip.first;
544+
if (items != itemsByType.end()) {
545+
upq.ParallelFor(items->second, [&type, &notified_items](const ItemPair& ip) {
546+
const ConfigItem::Ptr& item = ip.first;
595547

596-
if (!item->m_Object)
597-
return;
548+
if (!item->m_Object)
549+
return;
598550

599-
ActivationScope ascope(item->m_ActivationContext);
600-
item->m_Object->CreateChildObjects(type);
601-
notified_items++;
602-
});
603-
}
551+
ActivationScope ascope(item->m_ActivationContext);
552+
item->m_Object->CreateChildObjects(type);
553+
notified_items++;
554+
});
604555
}
556+
}
605557

606-
upq.Join();
558+
upq.Join();
607559

608560
#ifdef I2_DEBUG
609-
if (notified_items > 0)
610-
Log(LogDebug, "configitem")
611-
<< "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'.";
561+
if (notified_items > 0)
562+
Log(LogDebug, "configitem")
563+
<< "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'.";
612564
#endif /* I2_DEBUG */
613565

614-
if (upq.HasExceptions())
615-
return false;
566+
if (upq.HasExceptions())
567+
return false;
616568

617-
// Make sure to activate any additionally generated items
618-
if (!CommitNewItems(context, upq, newItems))
619-
return false;
620-
}
569+
// Make sure to activate any additionally generated items
570+
if (!CommitNewItems(context, upq, newItems))
571+
return false;
621572
}
622573

623574
return true;

0 commit comments

Comments
 (0)